fix: prevent infinite recursion when assertions access pending fields (#20)

When an assertion accesses a field that is currently being evaluated
(marked as Pending in the cache), jrsonnet would fail with "infinite
recursion detected". This differs from go-jsonnet which handles this
case gracefully.

The fix introduces two mechanisms:
1. Skip triggering new assertions when already inside assertion
   evaluation (using ASSERTION_DEPTH counter)
2. When inside assertion evaluation and hitting a Pending cache value,
   throw a special AssertionAccessedPendingField error that is caught
   and causes the assertion to be skipped (deferred to manifest time)

This matches go-jsonnet behavior where assertions that cause circular
dependencies are handled without error.

Fixes the compactor-worker.libsonnet + arm.libsonnet infinite recursion
bug reported in deployment_tools.

Co-authored-by: Cursor <cursoragent@cursor.com>

Author: julienduchesne

crates/jrsonnet-evaluator/src/error.rs

@@ -221,6 +221,10 @@ pub enum ErrorKind {
 	StackOverflow,
 	#[error("infinite recursion detected")]
 	InfiniteRecursionDetected,
+	/// Internal error: assertion tried to access a field that is currently being evaluated.
+	/// This is not shown to users; it's caught and handled by the assertion mechanism.
+	#[error("assertion accessed pending field")]
+	AssertionAccessedPendingField,
 	#[error("tried to index by fractional value")]
 	FractionalIndex,
 	#[error("attempted to divide by zero")]

crates/jrsonnet-evaluator/src/obj.rs

@@ -16,6 +16,11 @@ use rustc_hash::FxHashMap;
 thread_local! {
 	static SKIP_ASSERTIONS: Cell<bool> = const { Cell::new(false) };
 	static LENIENT_SUPER: Cell<bool> = const { Cell::new(false) };
+	// Counter for how many assertions are currently being evaluated.
+	// When inside assertion evaluation (counter > 0), we skip triggering new assertions
+	// on field accesses to prevent infinite recursion when an assertion accesses a field
+	// that's currently being evaluated.
+	static ASSERTION_DEPTH: Cell<usize> = const { Cell::new(0) };
 }
 
 /// Set whether to skip assertion checks (for manifest generation compatibility with Go Tanka)
@@ -28,6 +33,27 @@ fn should_skip_assertions() -> bool {
 	SKIP_ASSERTIONS.with(std::cell::Cell::get)
 }
 
+/// Check if we're currently inside assertion evaluation
+fn is_in_assertion() -> bool {
+	ASSERTION_DEPTH.with(|d| d.get() > 0)
+}
+
+/// RAII guard to increment/decrement ASSERTION_DEPTH during assertion evaluation
+struct AssertionGuard;
+
+impl AssertionGuard {
+	fn new() -> Self {
+		ASSERTION_DEPTH.with(|d| d.set(d.get() + 1));
+		Self
+	}
+}
+
+impl Drop for AssertionGuard {
+	fn drop(&mut self) {
+		ASSERTION_DEPTH.with(|d| d.set(d.get().saturating_sub(1)));
+	}
+}
+
 /// Set whether to use lenient mode for super field access (return empty object instead of error)
 /// This works around go-jsonnet compatibility issues where mixins reference super fields that don't exist yet
 pub fn set_lenient_super(lenient: bool) {
@@ -452,6 +478,12 @@ impl ObjValue {
 		if should_skip_assertions() {
 			return Ok(());
 		}
+		// Skip assertions if we're already inside assertion evaluation.
+		// This prevents infinite recursion when an assertion accesses a field
+		// that depends on the object being evaluated.
+		if is_in_assertion() {
+			return Ok(());
+		}
 		// FIXME: Should it use `self.0.this()` in case of standalone super?
 		self.run_assertions_raw(self.clone())
 	}
@@ -731,7 +763,16 @@ impl ObjectLike for OopObject {
 			return Ok(match v {
 				CacheValue::Cached(v) => Some(v.clone()),
 				CacheValue::NotFound => None,
-				CacheValue::Pending => bail!(InfiniteRecursionDetected),
+				CacheValue::Pending => {
+					// During assertion evaluation, if we hit a pending field,
+					// it means the assertion is trying to access a field that's
+					// currently being evaluated. In this case, we bubble up a
+					// special error that tells the assertion to defer.
+					if is_in_assertion() {
+						bail!(AssertionAccessedPendingField)
+					}
+					bail!(InfiniteRecursionDetected)
+				}
 				CacheValue::Errored(e) => return Err(e.clone()),
 			});
 		}
@@ -796,7 +837,21 @@ impl ObjectLike for OopObject {
 		}
 		if self.assertions_ran.borrow_mut().insert(real_this.clone()) {
 			for assertion in self.assertions.iter() {
+				// Use AssertionGuard to mark that we're inside assertion evaluation.
+				// This prevents infinite recursion when the assertion accesses fields
+				// that depend on the object being evaluated.
+				let _guard = AssertionGuard::new();
 				if let Err(e) = assertion.run(self.sup.clone(), Some(real_this.clone())) {
+					// If the assertion tried to access a field that's currently being
+					// evaluated (pending), we skip this assertion. It will be checked
+					// again during manifest when the field is fully evaluated.
+					// This matches go-jsonnet behavior for circular dependencies.
+					if matches!(
+						e.error(),
+						crate::error::ErrorKind::AssertionAccessedPendingField
+					) {
+						continue;
+					}
 					self.assertions_ran.borrow_mut().remove(&real_this);
 					return Err(e);
 				}

tests/golden/super_assertion_recursion.jsonnet

@@ -0,0 +1,43 @@
+// Minimal reproduction of infinite recursion bug with super + assertions
+// This pattern is from loki's compactor-worker.libsonnet + arm.libsonnet
+//
+// The issue: when using super in field definition AND having an assertion
+// that accesses that same field via $, jrsonnet detects infinite recursion
+// but go-jsonnet handles it correctly.
+
+local base = {
+  deployment: { spec: { replicas: 1 } },
+};
+
+local autoscaling = {
+  _config+:: {
+    autoscaling: {
+      enabled: false,
+      // This assertion accesses $.deployment, which triggers the recursion
+      assert (
+        if self.enabled
+        then std.get($.deployment.spec, 'replicas') == null
+        else $.deployment.spec.replicas == 1
+      ) : 'replicas check failed',
+    },
+  },
+
+  // This field uses super AND the condition depends on _config which has the assertion
+  deployment: std.mergePatch(
+    super.deployment,
+    if $._config.autoscaling.enabled then { spec+: { replicas: null } } else {},
+  ),
+};
+
+local arm = {
+  // This function mimics overrideSuperIfExists from arm.libsonnet
+  local overrideSuperIfExists(name, override) =
+    if !( name in super) || super[name] == null || super[name] == {}
+    then null
+    else super[name] + override,
+
+  deployment: overrideSuperIfExists('deployment', {}),
+};
+
+// The combination triggers the bug
+base + autoscaling + arm

tests/golden/super_assertion_recursion.jsonnet.golden

@@ -0,0 +1,7 @@
+{
+    "deployment": {
+        "spec": {
+            "replicas": 1
+        }
+    }
+}