fix(engine): cycle-guard same-law internal references in service layer (#788)

* fix(engine): cycle-guard same-law internal references in service layer

The internal-reference branch in resolve_inputs_with_service (source:
{ output: X } without a regulation) recursively evaluated the referenced
article without any cycle detection, unlike every other reference type
(external cross-law inputs, hooks, overrides, open terms). A law where
two articles source each other's outputs recursed unbounded and aborted
the whole process with a stack overflow (exit 134).

Guard the branch with the same resolution-context pattern used by the
other paths: check is_visited with a distinct "internal:" key (law id +
output name, NUL-separated), return EngineError::CircularReference on a
cycle, and enter/leave around the recursive evaluation so the guard is
released on both success and error paths.

Adds service-level regression tests for a direct two-article cycle and
an indirect three-article cycle (existing cycle tests only covered
ArticleEngine, which has its own visited-set guard).

* fix(engine): bound internal-reference recursion depth; add diamond regression test

The internal-reference branch was bounded only by the visited set
(distinct outputs), so a pathological non-cyclic law with a long chain
of internal references could still overflow the stack. Add the same
MAX_CROSS_LAW_DEPTH check used by evaluate_law_output_internal, so
internal references consume the shared cross-law depth budget.

Tests:
- diamond regression: two inputs sourcing the SAME output of another
  article must succeed (no false-positive cycle detection)
- deep chain: a linear internal chain longer than MAX_CROSS_LAW_DEPTH
  returns the depth-exceeded error instead of recursing

* fix(engine): emit structured warn when internal-reference depth limit fires

Author: tdjager

packages/engine/src/service.rs

@@ -2045,6 +2045,21 @@ impl LawExecutionService {
                 res_ctx
                     .trace_set_message(format!("Internal reference: {}#{}", law.id, output_name));
 
+                // Cycle detection: check if this internal output is already being
+                // resolved. Use \0 as separator to prevent key collisions, and an
+                // "internal:" prefix to keep keys distinct from external references.
+                let internal_key = format!("internal:{}\0{}", law.id, output_name);
+                if res_ctx.is_visited(&internal_key) {
+                    res_ctx.trace_set_message(format!(
+                        "Circular internal reference detected: {}#{} is already being resolved",
+                        law.id, output_name
+                    ));
+                    return Err(EngineError::CircularReference(format!(
+                        "Circular internal reference: output '{}' in {} is already being resolved",
+                        output_name, law.id
+                    )));
+                }
+
                 let ref_article = match law.find_article_by_output(output_name) {
                     Some(a) => a,
                     None => {
@@ -2060,14 +2075,49 @@ impl LawExecutionService {
                 };
 
                 let ref_params = parameters.clone();
-                let result = match self.evaluate_article_with_service(
+
+                // Enter internal resolution scope for cycle detection
+                res_ctx.enter(internal_key.clone());
+
+                // Check depth limit (mirrors evaluate_law_output_internal): the
+                // visited set bounds distinct outputs, but a long non-cyclic
+                // internal chain could still overflow the stack.
+                if res_ctx.depth > config::MAX_CROSS_LAW_DEPTH {
+                    res_ctx.leave(&internal_key);
+                    tracing::warn!(
+                        law_id = %law.id,
+                        output = %output_name,
+                        depth = res_ctx.depth,
+                        "Cross-law resolution depth exceeded (internal reference)"
+                    );
+                    res_ctx.trace_set_message(format!(
+                        "Cross-law resolution depth exceeded {} levels ({}:{})",
+                        config::MAX_CROSS_LAW_DEPTH,
+                        law.id,
+                        output_name
+                    ));
+                    return Err(EngineError::CircularReference(format!(
+                        "Cross-law resolution depth exceeded {} levels. \
+                         Possible circular reference involving {}:{}",
+                        config::MAX_CROSS_LAW_DEPTH,
+                        law.id,
+                        output_name
+                    )));
+                }
+
+                let eval_result = self.evaluate_article_with_service(
                     ref_article,
                     law,
                     ref_params,
                     Some(output_name),
                     "BESLUIT",
                     res_ctx,
-                ) {
+                );
+
+                // Leave scope (even on error, for correct cycle tracking)
+                res_ctx.leave(&internal_key);
+
+                let result = match eval_result {
                     Ok(r) => r,
                     Err(e) => {
                         res_ctx.trace_set_message(format!("Internal reference failed: {}", e));
@@ -2617,6 +2667,269 @@ articles:
         );
     }
 
+    #[test]
+    fn test_service_internal_circular_reference() {
+        // Same-law cycle: article 1's input sources article 2's output,
+        // and article 2's input sources article 1's output.
+        // Previously this recursed unbounded and crashed with a stack overflow.
+        let law = r#"
+$id: internal_cycle_law
+regulatory_layer: WET
+publication_date: '2025-01-01'
+articles:
+  - number: '1'
+    text: References output of article 2
+    machine_readable:
+      execution:
+        input:
+          - name: from_b
+            type: number
+            source:
+              output: output_b
+        output:
+          - name: output_a
+            type: number
+        actions:
+          - output: output_a
+            value: $from_b
+  - number: '2'
+    text: References output of article 1
+    machine_readable:
+      execution:
+        input:
+          - name: from_a
+            type: number
+            source:
+              output: output_a
+        output:
+          - name: output_b
+            type: number
+        actions:
+          - output: output_b
+            value: $from_a
+"#;
+
+        let mut service = LawExecutionService::new();
+        service.load_law(law).unwrap();
+
+        let result = service.evaluate_law_output(
+            "internal_cycle_law",
+            "output_a",
+            BTreeMap::new(),
+            "2025-01-01",
+        );
+
+        assert!(
+            matches!(result, Err(EngineError::CircularReference(_))),
+            "Expected CircularReference error, got: {:?}",
+            result
+        );
+    }
+
+    #[test]
+    fn test_service_internal_circular_reference_indirect() {
+        // Indirect same-law cycle across three articles:
+        // article 1 -> article 2 -> article 3 -> article 1.
+        let law = r#"
+$id: internal_cycle_law_3
+regulatory_layer: WET
+publication_date: '2025-01-01'
+articles:
+  - number: '1'
+    text: References output of article 2
+    machine_readable:
+      execution:
+        input:
+          - name: from_b
+            type: number
+            source:
+              output: output_b
+        output:
+          - name: output_a
+            type: number
+        actions:
+          - output: output_a
+            value: $from_b
+  - number: '2'
+    text: References output of article 3
+    machine_readable:
+      execution:
+        input:
+          - name: from_c
+            type: number
+            source:
+              output: output_c
+        output:
+          - name: output_b
+            type: number
+        actions:
+          - output: output_b
+            value: $from_c
+  - number: '3'
+    text: References output of article 1
+    machine_readable:
+      execution:
+        input:
+          - name: from_a
+            type: number
+            source:
+              output: output_a
+        output:
+          - name: output_c
+            type: number
+        actions:
+          - output: output_c
+            value: $from_a
+"#;
+
+        let mut service = LawExecutionService::new();
+        service.load_law(law).unwrap();
+
+        let result = service.evaluate_law_output(
+            "internal_cycle_law_3",
+            "output_a",
+            BTreeMap::new(),
+            "2025-01-01",
+        );
+
+        assert!(
+            matches!(result, Err(EngineError::CircularReference(_))),
+            "Expected CircularReference error, got: {:?}",
+            result
+        );
+    }
+
+    #[test]
+    fn test_service_internal_diamond_reference_succeeds() {
+        // Legitimate diamond, no cycle: two inputs of article 1 both source
+        // the SAME output of article 2. The cycle guard must not produce a
+        // false positive here (the visited key is released between siblings).
+        let law = r#"
+$id: internal_diamond_law
+regulatory_layer: WET
+publication_date: '2025-01-01'
+articles:
+  - number: '1'
+    text: Sums the same output of article 2 twice
+    machine_readable:
+      execution:
+        input:
+          - name: left
+            type: number
+            source:
+              output: shared_value
+          - name: right
+            type: number
+            source:
+              output: shared_value
+        output:
+          - name: combined
+            type: number
+        actions:
+          - output: combined
+            operation: ADD
+            values:
+              - $left
+              - $right
+  - number: '2'
+    text: Provides a shared value
+    machine_readable:
+      execution:
+        output:
+          - name: shared_value
+            type: number
+        actions:
+          - output: shared_value
+            value: 21
+"#;
+
+        let mut service = LawExecutionService::new();
+        service.load_law(law).unwrap();
+
+        let result = service
+            .evaluate_law_output(
+                "internal_diamond_law",
+                "combined",
+                BTreeMap::new(),
+                "2025-01-01",
+            )
+            .expect("diamond-shaped internal references must not be flagged as circular");
+
+        assert_eq!(result.outputs.get("combined"), Some(&Value::Int(42)));
+    }
+
+    #[test]
+    fn test_service_internal_deep_chain_depth_limit() {
+        // A linear (non-cyclic) chain of internal references longer than
+        // MAX_CROSS_LAW_DEPTH must hit the depth limit instead of recursing
+        // until the stack overflows.
+        let chain_len = config::MAX_CROSS_LAW_DEPTH + 5;
+        let mut yaml = String::from(
+            "$id: internal_deep_chain_law\n\
+             regulatory_layer: WET\n\
+             publication_date: '2025-01-01'\n\
+             articles:\n",
+        );
+        for i in 0..chain_len {
+            yaml.push_str(&format!(
+                r#"  - number: '{num}'
+    text: Chain link {num}
+    machine_readable:
+      execution:
+        input:
+          - name: from_next
+            type: number
+            source:
+              output: value_{next}
+        output:
+          - name: value_{num}
+            type: number
+        actions:
+          - output: value_{num}
+            value: $from_next
+"#,
+                num = i,
+                next = i + 1
+            ));
+        }
+        // Terminal article: a literal value, no further references.
+        yaml.push_str(&format!(
+            r#"  - number: '{num}'
+    text: Chain terminal
+    machine_readable:
+      execution:
+        output:
+          - name: value_{num}
+            type: number
+        actions:
+          - output: value_{num}
+            value: 1
+"#,
+            num = chain_len
+        ));
+
+        let mut service = LawExecutionService::new();
+        service.load_law(&yaml).unwrap();
+
+        let result = service.evaluate_law_output(
+            "internal_deep_chain_law",
+            "value_0",
+            BTreeMap::new(),
+            "2025-01-01",
+        );
+
+        match result {
+            Err(EngineError::CircularReference(msg)) => {
+                assert!(
+                    msg.contains("depth exceeded"),
+                    "Expected depth-exceeded error, got: {}",
+                    msg
+                );
+            }
+            other => panic!("Expected CircularReference depth error, got: {:?}", other),
+        }
+    }
+
     // -------------------------------------------------------------------------
     // Parameter Override Tests
     // -------------------------------------------------------------------------