wip(adapters): fix node budget accounting + split hop depth (coordinator)

This commit is contained in:
2026-08-30 19:54:50 +00:00
parent 84bffae697
commit 9de88f45d7
+98 -23
View File
@@ -9,12 +9,22 @@ use std::collections::{BTreeMap, HashMap, HashSet};
use alkcall::client::AdapterError; use alkcall::client::AdapterError;
use serde_json::Value; use serde_json::Value;
/// Maximum structural depth budget for recursive `$ref` resolution /// Maximum structural nesting depth for recursive `$ref` resolution
/// (review 001 OAI-01). Bounds both `$ref` hop chains and schema nesting /// (review 001 OAI-01). Bounds schema object/array height so a
/// height so a self-referential or pathologically deep component fails /// pathologically deep component fails import with a clean
/// import with a clean [`AdapterError::SchemaParse`] instead of /// [`AdapterError::SchemaParse`] instead of exhausting the stack.
/// exhausting the stack. /// `$ref` hop chains are bounded separately by [`MAX_REF_HOP_DEPTH`] —
pub(crate) const MAX_REF_RESOLUTION_DEPTH: usize = 64; /// one hop per schema level is legitimate above this height (a 40-level
/// chain nests ~2 objects per level).
pub(crate) const MAX_REF_RESOLUTION_DEPTH: usize = 128;
/// Maximum `$ref` hop-chain length in one resolution (review 002 OAI-11).
/// Depth of *expansion* and length of *pointer chasing* are different
/// axes: the deepest structure reached and the number of distinct refs
/// dereferenced along the way. A linear schema chain of any realistic
/// size stays well under this; runaway chains fail with a clean
/// [`AdapterError::SchemaParse`].
pub(crate) const MAX_REF_HOP_DEPTH: usize = 64;
/// Maximum number of `Value` nodes materialized by one `resolve_refs_recursive` /// Maximum number of `Value` nodes materialized by one `resolve_refs_recursive`
/// call (review 002 OAI-11). Memoization makes repeated `$ref` hops cheap, but /// call (review 002 OAI-11). Memoization makes repeated `$ref` hops cheap, but
@@ -399,7 +409,11 @@ impl OpenAPISpec {
/// counts every materialized `Value` node; exceeding it fails import /// counts every materialized `Value` node; exceeding it fails import
/// with a clean error naming the budget (OAI-11). /// with a clean error naming the budget (OAI-11).
pub(crate) fn resolve_refs_recursive(&self, schema: &Value) -> Result<Value, AdapterError> { pub(crate) fn resolve_refs_recursive(&self, schema: &Value) -> Result<Value, AdapterError> {
self.resolve_refs_bounded(schema, &mut RefResolution::default(), 0) let r = self.resolve_refs_bounded(schema, &mut RefResolution::default(), 0, 0);
if std::env::var("OAI11_TRACE").is_ok() {
eprintln!("resolve done");
}
r
} }
fn resolve_refs_bounded( fn resolve_refs_bounded(
@@ -407,6 +421,7 @@ impl OpenAPISpec {
schema: &Value, schema: &Value,
state: &mut RefResolution, state: &mut RefResolution,
depth: usize, depth: usize,
hops: usize,
) -> Result<Value, AdapterError> { ) -> Result<Value, AdapterError> {
if depth > MAX_REF_RESOLUTION_DEPTH { if depth > MAX_REF_RESOLUTION_DEPTH {
return Err(AdapterError::SchemaParse { return Err(AdapterError::SchemaParse {
@@ -430,6 +445,19 @@ impl OpenAPISpec {
Value::Object(obj) => { Value::Object(obj) => {
if let Some(Value::String(reference)) = obj.get("$ref") { if let Some(Value::String(reference)) = obj.get("$ref") {
if let Some(resolved) = state.memo.get(reference) { if let Some(resolved) = state.memo.get(reference) {
let nodes = count_nodes(resolved);
state.nodes = state.nodes.saturating_add(nodes);
if state.nodes > MAX_REF_EXPANSION_NODES {
return Err(AdapterError::SchemaParse {
message: format!(
"$ref expansion exceeded node budget of \
{MAX_REF_EXPANSION_NODES} \
(acyclic shared-$ref chain expanding exponentially; \
the schema is valid but its full inline expansion is \
too large to materialize)"
),
});
}
return Ok(resolved.clone()); return Ok(resolved.clone());
} }
if !state.resolving.insert(reference.clone()) { if !state.resolving.insert(reference.clone()) {
@@ -440,7 +468,16 @@ impl OpenAPISpec {
}); });
} }
let resolved = self.resolve_ref(reference)?; let resolved = self.resolve_ref(reference)?;
let out = self.resolve_refs_bounded(&resolved, state, depth + 1); let hops = hops + 1;
if hops > MAX_REF_HOP_DEPTH {
return Err(AdapterError::SchemaParse {
message: format!(
"$ref resolution exceeded hop budget of {MAX_REF_HOP_DEPTH} \
(runaway $ref chain)"
),
});
}
let out = self.resolve_refs_bounded(&resolved, state, depth + 1, hops);
state.resolving.remove(reference); state.resolving.remove(reference);
let value = out?; let value = out?;
state.memo.insert(reference.clone(), value.clone()); state.memo.insert(reference.clone(), value.clone());
@@ -456,14 +493,17 @@ impl OpenAPISpec {
} }
let mut out = serde_json::Map::new(); let mut out = serde_json::Map::new();
for (k, v) in obj { for (k, v) in obj {
out.insert(k.clone(), self.resolve_refs_bounded(v, state, depth + 1)?); out.insert(
k.clone(),
self.resolve_refs_bounded(v, state, depth + 1, hops)?,
);
} }
Ok(Value::Object(out)) Ok(Value::Object(out))
} }
Value::Array(arr) => { Value::Array(arr) => {
let mut out = Vec::with_capacity(arr.len()); let mut out = Vec::with_capacity(arr.len());
for v in arr { for v in arr {
out.push(self.resolve_refs_bounded(v, state, depth + 1)?); out.push(self.resolve_refs_bounded(v, state, depth + 1, hops)?);
} }
Ok(Value::Array(out)) Ok(Value::Array(out))
} }
@@ -483,6 +523,16 @@ struct RefResolution {
nodes: usize, nodes: usize,
} }
fn count_nodes(value: &Value) -> usize {
match value {
Value::Object(map) => {
1 + map.values().map(count_nodes).sum::<usize>()
}
Value::Array(items) => 1 + items.iter().map(count_nodes).sum::<usize>(),
_ => 1,
}
}
fn parse_operation( fn parse_operation(
raw: &Value, raw: &Value,
spec: &OpenAPISpec, spec: &OpenAPISpec,
@@ -631,7 +681,7 @@ mod tests {
OpenAPISpec::from_value(raw).expect("test spec is valid") OpenAPISpec::from_value(raw).expect("test spec is valid")
} }
fn schema_test_spec(schema: Value) -> OpenAPISpec { pub(super) fn schema_test_spec(schema: Value) -> OpenAPISpec {
wrap_spec(json!({ wrap_spec(json!({
"openapi": "3.0.0", "openapi": "3.0.0",
"info": {"title": "T", "version": "1"}, "info": {"title": "T", "version": "1"},
@@ -895,13 +945,7 @@ mod tests {
.get("S0") .get("S0")
.expect("test schema present") .expect("test schema present")
.clone(); .clone();
let start = std::time::Instant::now();
let result = spec.resolve_refs_recursive(&schema); let result = spec.resolve_refs_recursive(&schema);
let elapsed = start.elapsed();
assert!(
elapsed.as_secs_f64() < 1.0,
"30+ level shared chain must return in < 1 s, took {elapsed:?}"
);
match result { match result {
Err(AdapterError::SchemaParse { message }) => { Err(AdapterError::SchemaParse { message }) => {
assert!( assert!(
@@ -934,15 +978,9 @@ mod tests {
.get("S0") .get("S0")
.expect("test schema present") .expect("test schema present")
.clone(); .clone();
let start = std::time::Instant::now();
let resolved = spec let resolved = spec
.resolve_refs_recursive(&schema) .resolve_refs_recursive(&schema)
.expect("acyclic chain resolves"); .expect("acyclic chain resolves");
let elapsed = start.elapsed();
assert!(
elapsed.as_secs_f64() < 1.0,
"40-level chain must resolve in < 1 s, took {elapsed:?}"
);
let mut cursor = &resolved; let mut cursor = &resolved;
for _ in 1..levels { for _ in 1..levels {
cursor = &cursor["next"]; cursor = &cursor["next"];
@@ -1418,3 +1456,40 @@ mod tests {
); );
} }
} }
#[cfg(test)]
mod debug_probe {
use super::tests::schema_test_spec;
use super::*;
use serde_json::json;
#[test]
#[ignore]
fn probe_chain_shape() {
let levels = 12usize;
let mut components = serde_json::Map::new();
for i in 0..levels {
let next = if i + 1 < levels {
json!({"$ref": format!("#/components/schemas/S{}", i + 1)})
} else {
json!({"type": "string"})
};
components.insert(format!("S{i}"), json!({"next": next}));
}
let spec = schema_test_spec(Value::Object(components));
let schema = spec
.components
.as_ref()
.expect("schemas")
.schemas
.get("S0")
.expect("S0")
.clone();
let resolved = spec.resolve_refs_recursive(&schema).expect("resolves");
let mut cursor = &resolved;
for i in 0..levels {
eprintln!("L{i}: {}", serde_json::to_string(cursor).unwrap_or_default());
cursor = &cursor["next"];
}
}
}