Skip to main content

octl_core/
report.rs

1//! §7.3 terminal-report payload validation (design.md §7.3).
2//!
3//! Validates the structural shape of a `node.report` payload — `success`
4//! required; optional `summary`, `cancelled`/`reason`, `discussion_items`,
5//! `spinoff_proposals`, `wrap_up_recommendations` — before the reducer
6//! ever projects it. Lives in `octl-core` (not the CLI) so the supervisor
7//! can validate child reports with the same rules it would consume
8//! (design.md §7.3 step 3), rather than copying the validator or depending
9//! on the CLI crate.
10//!
11//! Errors are domain-typed ([`ReportValidationError`]); the CLI maps them
12//! to its `CliError` envelope at the boundary.
13
14use serde_json::Value;
15
16use crate::schema::Kind;
17
18/// A §7.3 report payload failed structural validation.
19///
20/// Every variant describes one schema violation. The CLI renders these as
21/// a `schema_violation` error; [`ReportValidationError::expected`] supplies
22/// the machine-readable `expected` hint for the variants that carry one.
23#[derive(Debug, thiserror::Error)]
24pub enum ReportValidationError {
25    /// The payload root was not a JSON object.
26    #[error("report payload must be a JSON object")]
27    NotObject,
28
29    /// The required `success` field was absent.
30    #[error("report payload missing required field `success`")]
31    MissingSuccess,
32
33    /// `success` was present but not a boolean.
34    #[error("field `success` must be a boolean")]
35    SuccessNotBoolean,
36
37    /// `summary` was present but not a string (or null).
38    #[error("field `summary` must be a string")]
39    SummaryNotString,
40
41    /// `cancelled` was present but not a boolean.
42    #[error("field `cancelled` must be a boolean")]
43    CancelledNotBoolean,
44
45    /// `reason` was present but not a string.
46    #[error("field `reason` must be a string")]
47    ReasonNotString,
48
49    /// `cancelled: true` was paired with `success: true` (§7.7 forbids it).
50    #[error("`cancelled: true` requires `success: false`")]
51    CancelledRequiresSuccessFalse,
52
53    /// `cancelled: true` lacked a non-empty `reason` string (§7.7).
54    #[error("`cancelled: true` requires a non-empty `reason` string")]
55    CancelledRequiresReason,
56
57    /// `discussion_items` was present but not an array.
58    #[error("field `discussion_items` must be an array")]
59    DiscussionItemsNotArray,
60
61    /// A `discussion_items` element was not a JSON object.
62    #[error("discussion_items[{index}] must be a JSON object")]
63    DiscussionItemNotObject {
64        /// Index of the offending element.
65        index: usize,
66    },
67
68    /// A `discussion_items` element lacked a non-empty `topic` string.
69    #[error("discussion_items[{index}].topic must be a non-empty string")]
70    DiscussionItemTopicMissing {
71        /// Index of the offending element.
72        index: usize,
73    },
74
75    /// A `discussion_items` element's `severity` was not a string.
76    #[error("discussion_items[{index}].severity must be a string")]
77    DiscussionItemSeverityNotString {
78        /// Index of the offending element.
79        index: usize,
80    },
81
82    /// `spinoff_proposals` was present but not an array.
83    #[error("field `spinoff_proposals` must be an array")]
84    SpinoffProposalsNotArray,
85
86    /// A `spinoff_proposals` element was not a JSON object.
87    #[error("spinoff_proposals[{index}] must be a JSON object")]
88    SpinoffProposalNotObject {
89        /// Index of the offending element.
90        index: usize,
91    },
92
93    /// A `spinoff_proposals` element lacked a non-empty `proposed_title`.
94    #[error("spinoff_proposals[{index}].proposed_title must be a non-empty string")]
95    SpinoffProposalTitleMissing {
96        /// Index of the offending element.
97        index: usize,
98    },
99
100    /// A `spinoff_proposals` element's `proposed_kind` was not a string.
101    #[error("spinoff_proposals[{index}].proposed_kind must be a string")]
102    SpinoffProposalKindNotString {
103        /// Index of the offending element.
104        index: usize,
105    },
106
107    /// A `spinoff_proposals` element's `proposed_kind` was not a known [`Kind`].
108    #[error("spinoff_proposals[{index}].proposed_kind `{kind}` is not a known kind")]
109    SpinoffProposalKindUnknown {
110        /// Index of the offending element.
111        index: usize,
112        /// The rejected kind string.
113        kind: String,
114    },
115
116    /// A `spinoff_proposals` element's `rationale` was not a string (or null).
117    #[error("spinoff_proposals[{index}].rationale must be a string")]
118    SpinoffProposalRationaleNotString {
119        /// Index of the offending element.
120        index: usize,
121    },
122
123    /// A declared string-array field was not an array.
124    #[error("field `{field}` must be an array")]
125    FieldNotArray {
126        /// The offending field name.
127        field: String,
128    },
129
130    /// An element of a declared string-array field was not a string.
131    #[error("{field}[{index}] must be a string")]
132    FieldElementNotString {
133        /// The offending field name.
134        field: String,
135        /// Index of the offending element.
136        index: usize,
137    },
138
139    /// A nested path expected to hold a string array was not an array.
140    #[error("{path} must be an array")]
141    PathNotArray {
142        /// Dotted/indexed path to the offending value.
143        path: String,
144    },
145
146    /// An element at a nested string-array path was not a string.
147    #[error("{path}[{index}] must be a string")]
148    PathElementNotString {
149        /// Dotted/indexed path to the offending array.
150        path: String,
151        /// Index of the offending element.
152        index: usize,
153    },
154}
155
156impl ReportValidationError {
157    /// The machine-readable `expected` hint for this error, if any.
158    ///
159    /// Mirrors the `with_expected(...)` payloads the CLI previously
160    /// attached inline, so callers can surface the same structured hint.
161    #[must_use]
162    pub fn expected(&self) -> Option<Value> {
163        match self {
164            Self::MissingSuccess | Self::SuccessNotBoolean => {
165                Some(serde_json::json!({"field": "success", "type": "boolean"}))
166            }
167            // Source the accepted kinds from the enum so the hint can never
168            // drift from what the validator actually accepts (see
169            // `Kind::WIRE_NAMES` and its serde round-trip test).
170            Self::SpinoffProposalKindUnknown { .. } => Some(serde_json::json!(Kind::WIRE_NAMES)),
171            _ => None,
172        }
173    }
174}
175
176/// Validate a §7.3 report payload's structural shape.
177///
178/// Rejects anything obviously not a report before the reducer ever sees
179/// it, so the caller can name the offending field instead of bubbling a
180/// generic `CorruptEventLog`. Keeps the current validation logic verbatim;
181/// this is a relocation, not a tightening.
182///
183/// # Errors
184///
185/// Returns a [`ReportValidationError`] describing the first schema
186/// violation found.
187pub fn validate_report_payload(data: &Value) -> Result<(), ReportValidationError> {
188    let obj = data.as_object().ok_or(ReportValidationError::NotObject)?;
189
190    // `success` is the one strictly required field per §7.3. A cancel-
191    // synthesized report (§7.7) may carry `cancelled: true` AND
192    // `success: false` — both are still booleans on the wire.
193    let success = obj
194        .get("success")
195        .ok_or(ReportValidationError::MissingSuccess)?;
196    if !success.is_boolean() {
197        return Err(ReportValidationError::SuccessNotBoolean);
198    }
199
200    if let Some(v) = obj.get("summary") {
201        if !v.is_string() && !v.is_null() {
202            return Err(ReportValidationError::SummaryNotString);
203        }
204    }
205    let cancelled = match obj.get("cancelled") {
206        None | Some(Value::Null) => false,
207        Some(v) => v
208            .as_bool()
209            .ok_or(ReportValidationError::CancelledNotBoolean)?,
210    };
211    let reason = match obj.get("reason") {
212        None | Some(Value::Null) => None,
213        Some(v) => Some(v.as_str().ok_or(ReportValidationError::ReasonNotString)?),
214    };
215
216    // §7.7: a cancel-synthesized report carries `cancelled: true,
217    // success: false, reason: <non-empty>`. Allowing `success: true`
218    // alongside `cancelled: true` would persist a contradiction (the
219    // reducer prioritizes `cancelled`, so the node would be cancelled
220    // while `last_report.success == true`).
221    if cancelled {
222        // `success` was confirmed a boolean above, so this never panics;
223        // `expect` documents that invariant rather than masking a reorder
224        // bug behind `unwrap_or(false)`.
225        if success
226            .as_bool()
227            .expect("success validated as boolean above")
228        {
229            return Err(ReportValidationError::CancelledRequiresSuccessFalse);
230        }
231        match reason {
232            Some(s) if !s.trim().is_empty() => {}
233            _ => return Err(ReportValidationError::CancelledRequiresReason),
234        }
235    }
236
237    validate_discussion_items(obj.get("discussion_items"))?;
238    validate_spinoff_proposals(obj.get("spinoff_proposals"))?;
239    validate_string_array(
240        obj.get("wrap_up_recommendations"),
241        "wrap_up_recommendations",
242    )?;
243    Ok(())
244}
245
246fn validate_discussion_items(v: Option<&Value>) -> Result<(), ReportValidationError> {
247    let arr = match v {
248        Some(Value::Array(a)) => a,
249        Some(_) => return Err(ReportValidationError::DiscussionItemsNotArray),
250        None => return Ok(()),
251    };
252    for (i, item) in arr.iter().enumerate() {
253        let obj = item
254            .as_object()
255            .ok_or(ReportValidationError::DiscussionItemNotObject { index: i })?;
256        let topic = obj.get("topic").and_then(Value::as_str);
257        if topic.is_none_or(|t| t.trim().is_empty()) {
258            return Err(ReportValidationError::DiscussionItemTopicMissing { index: i });
259        }
260        if let Some(sev) = obj.get("severity") {
261            if !sev.is_string() {
262                return Err(ReportValidationError::DiscussionItemSeverityNotString { index: i });
263            }
264            // §7.3 example lists "discuss|critical" but the design
265            // calls for forward-compatibility — accept any string and
266            // let the supervisor interpret unknown severities. (A
267            // CLI-side closed-set check would deadlock agents shipped
268            // ahead of a CLI release; see review #2/DeepSeek and #15
269            // /Claude.)
270        }
271        if let Some(opts) = obj.get("options") {
272            validate_string_array_at(opts, &format!("discussion_items[{i}].options"))?;
273        }
274    }
275    Ok(())
276}
277
278fn validate_spinoff_proposals(v: Option<&Value>) -> Result<(), ReportValidationError> {
279    let arr = match v {
280        Some(Value::Array(a)) => a,
281        Some(_) => return Err(ReportValidationError::SpinoffProposalsNotArray),
282        None => return Ok(()),
283    };
284    for (i, item) in arr.iter().enumerate() {
285        let obj = item
286            .as_object()
287            .ok_or(ReportValidationError::SpinoffProposalNotObject { index: i })?;
288        let title = obj.get("proposed_title").and_then(Value::as_str);
289        if title.is_none_or(|t| t.trim().is_empty()) {
290            return Err(ReportValidationError::SpinoffProposalTitleMissing { index: i });
291        }
292        let kind_str = obj
293            .get("proposed_kind")
294            .and_then(Value::as_str)
295            .ok_or(ReportValidationError::SpinoffProposalKindNotString { index: i })?;
296        // Reject unknown kinds at the boundary so the supervisor never
297        // has to translate a generic `CorruptEventLog` for the user.
298        // Mirrors the `Kind` enum's `rename_all = "kebab-case"` serde
299        // routing.
300        if serde_json::from_value::<Kind>(Value::String(kind_str.to_string())).is_err() {
301            return Err(ReportValidationError::SpinoffProposalKindUnknown {
302                index: i,
303                kind: kind_str.to_string(),
304            });
305        }
306        if let Some(rationale) = obj.get("rationale") {
307            if !rationale.is_string() && !rationale.is_null() {
308                return Err(ReportValidationError::SpinoffProposalRationaleNotString { index: i });
309            }
310        }
311    }
312    Ok(())
313}
314
315/// Path-aware string-array validator. Used for nested fields where the
316/// caller wants to embed an index in the error message.
317fn validate_string_array_at(v: &Value, path: &str) -> Result<(), ReportValidationError> {
318    let arr = v
319        .as_array()
320        .ok_or_else(|| ReportValidationError::PathNotArray {
321            path: path.to_string(),
322        })?;
323    for (i, item) in arr.iter().enumerate() {
324        if !item.is_string() {
325            return Err(ReportValidationError::PathElementNotString {
326                path: path.to_string(),
327                index: i,
328            });
329        }
330    }
331    Ok(())
332}
333
334fn validate_string_array(v: Option<&Value>, field: &str) -> Result<(), ReportValidationError> {
335    let arr = match v {
336        Some(Value::Array(a)) => a,
337        Some(_) => {
338            return Err(ReportValidationError::FieldNotArray {
339                field: field.to_string(),
340            })
341        }
342        None => return Ok(()),
343    };
344    for (i, item) in arr.iter().enumerate() {
345        if !item.is_string() {
346            return Err(ReportValidationError::FieldElementNotString {
347                field: field.to_string(),
348                index: i,
349            });
350        }
351    }
352    Ok(())
353}
354
355#[cfg(test)]
356mod tests {
357    use super::*;
358    use serde_json::json;
359
360    // --- valid payloads ---
361
362    #[test]
363    fn validates_minimal_success_payload() {
364        let v = json!({"success": true});
365        assert!(validate_report_payload(&v).is_ok());
366    }
367
368    #[test]
369    fn validates_full_success_payload() {
370        let v = json!({
371            "success": true,
372            "summary": "did the thing",
373            "discussion_items": [
374                {"topic": "naming", "severity": "discuss", "options": ["a", "b"]},
375            ],
376            "spinoff_proposals": [
377                {"proposed_title": "follow-up", "proposed_kind": "code", "rationale": "later"},
378            ],
379            "wrap_up_recommendations": ["rebase", "squash"],
380        });
381        assert!(validate_report_payload(&v).is_ok());
382    }
383
384    #[test]
385    fn discussion_item_unknown_severity_accepted_for_forward_compat() {
386        // Forward-compat: a supervisor may add new severities without a
387        // CLI release. The validator only enforces severity is a string.
388        let v = json!({
389            "success": true,
390            "discussion_items": [{"topic": "x", "severity": "info"}],
391        });
392        assert!(validate_report_payload(&v).is_ok());
393    }
394
395    #[test]
396    fn cancel_synthesized_report_shape_ok() {
397        // Mirror of run cancel's synthesized payload (run/cancel.rs).
398        let v = json!({
399            "success": false,
400            "cancelled": true,
401            "reason": "cancelled by user",
402            "summary": "Run cancelled before agent reported.",
403            "discussion_items": [],
404            "spinoff_proposals": [],
405            "wrap_up_recommendations": [],
406        });
407        assert!(validate_report_payload(&v).is_ok());
408    }
409
410    // --- invalid payloads ---
411
412    #[test]
413    fn non_object_root_rejected() {
414        let v = json!([1, 2, 3]);
415        assert!(matches!(
416            validate_report_payload(&v),
417            Err(ReportValidationError::NotObject)
418        ));
419    }
420
421    #[test]
422    fn missing_success_rejected() {
423        let v = json!({"summary": "no success field"});
424        let err = validate_report_payload(&v).unwrap_err();
425        assert!(matches!(err, ReportValidationError::MissingSuccess));
426        // Missing `success` carries the structured `expected` hint.
427        assert_eq!(
428            err.expected(),
429            Some(json!({"field": "success", "type": "boolean"}))
430        );
431    }
432
433    #[test]
434    fn success_variants_carry_field_type_hint() {
435        // Both `success` errors reproduce the exact CLI hint, byte-for-byte.
436        let hint = Some(json!({"field": "success", "type": "boolean"}));
437        assert_eq!(ReportValidationError::MissingSuccess.expected(), hint);
438        assert_eq!(ReportValidationError::SuccessNotBoolean.expected(), hint);
439    }
440
441    #[test]
442    fn summary_must_be_string() {
443        let v = json!({"success": true, "summary": 42});
444        assert!(matches!(
445            validate_report_payload(&v),
446            Err(ReportValidationError::SummaryNotString)
447        ));
448    }
449
450    #[test]
451    fn discussion_item_options_non_array_rejected() {
452        let v = json!({
453            "success": true,
454            "discussion_items": [{"topic": "x", "options": "not-an-array"}],
455        });
456        assert!(matches!(
457            validate_report_payload(&v),
458            Err(ReportValidationError::PathNotArray { .. })
459        ));
460    }
461
462    #[test]
463    fn cancelled_requires_non_whitespace_reason() {
464        let v = json!({"success": false, "cancelled": true, "reason": "   "});
465        assert!(matches!(
466            validate_report_payload(&v),
467            Err(ReportValidationError::CancelledRequiresReason)
468        ));
469    }
470
471    #[test]
472    fn non_boolean_success_rejected() {
473        let v = json!({"success": "yes"});
474        assert!(matches!(
475            validate_report_payload(&v),
476            Err(ReportValidationError::SuccessNotBoolean)
477        ));
478    }
479
480    #[test]
481    fn discussion_item_missing_topic_rejected() {
482        let v = json!({
483            "success": true,
484            "discussion_items": [{"severity": "discuss"}],
485        });
486        assert!(matches!(
487            validate_report_payload(&v),
488            Err(ReportValidationError::DiscussionItemTopicMissing { index: 0 })
489        ));
490    }
491
492    #[test]
493    fn discussion_item_non_string_severity_rejected() {
494        let v = json!({
495            "success": true,
496            "discussion_items": [{"topic": "x", "severity": 42}],
497        });
498        assert!(matches!(
499            validate_report_payload(&v),
500            Err(ReportValidationError::DiscussionItemSeverityNotString { index: 0 })
501        ));
502    }
503
504    #[test]
505    fn discussion_item_options_must_be_strings() {
506        let v = json!({
507            "success": true,
508            "discussion_items": [{"topic": "x", "options": [1, 2]}],
509        });
510        assert!(matches!(
511            validate_report_payload(&v),
512            Err(ReportValidationError::PathElementNotString { index: 0, .. })
513        ));
514    }
515
516    #[test]
517    fn spinoff_unknown_proposed_kind_rejected() {
518        let v = json!({
519            "success": true,
520            "spinoff_proposals": [{"proposed_title": "x", "proposed_kind": "not-a-kind"}],
521        });
522        let err = validate_report_payload(&v).unwrap_err();
523        assert!(matches!(
524            err,
525            ReportValidationError::SpinoffProposalKindUnknown { index: 0, .. }
526        ));
527        // Unknown kind surfaces the exact closed-set of known kinds, and
528        // that set is the enum's own wire names (no drift).
529        assert_eq!(err.expected(), Some(json!(crate::schema::Kind::WIRE_NAMES)));
530        assert_eq!(
531            err.expected(),
532            Some(json!([
533                "code",
534                "spinoff",
535                "orchestrated",
536                "research",
537                "technical-decision",
538                "make-skill",
539                "fan-out",
540                "bugfix",
541                "orchestrate",
542            ]))
543        );
544    }
545
546    #[test]
547    fn spinoff_missing_kind_rejected() {
548        let v = json!({
549            "success": true,
550            "spinoff_proposals": [{"proposed_title": "x"}],
551        });
552        assert!(matches!(
553            validate_report_payload(&v),
554            Err(ReportValidationError::SpinoffProposalKindNotString { index: 0 })
555        ));
556    }
557
558    #[test]
559    fn cancelled_requires_success_false() {
560        let v = json!({"success": true, "cancelled": true, "reason": "x"});
561        assert!(matches!(
562            validate_report_payload(&v),
563            Err(ReportValidationError::CancelledRequiresSuccessFalse)
564        ));
565    }
566
567    #[test]
568    fn cancelled_requires_reason() {
569        let v = json!({"success": false, "cancelled": true});
570        assert!(matches!(
571            validate_report_payload(&v),
572            Err(ReportValidationError::CancelledRequiresReason)
573        ));
574    }
575
576    #[test]
577    fn wrap_up_must_be_string_array() {
578        let v = json!({
579            "success": true,
580            "wrap_up_recommendations": ["ok", 42],
581        });
582        assert!(matches!(
583            validate_report_payload(&v),
584            Err(ReportValidationError::FieldElementNotString { index: 1, .. })
585        ));
586    }
587}