Fix deep workflow graph composition

[pinodeca]

Summary

• store Durofut children as opaque RawValue JSON objects so each graph level deserializes independently
• parse children on walker descent and validate graph depth before df.explain() traversal
• preserve PostgreSQL stack/resource errors in df.ensure_durofut() and update the 0.2.6 upgrade path
• add wire-format, composer, depth-boundary, operator, and 200-step execution regressions

Supersedes #328.
Fixes #327.

Validation

• cargo build --features pg17
• cargo clippy --features pg17 -- -D warnings
• cargo fmt -p pg_durable -- --check
• ./scripts/test-unit.sh (263 passed, 16 ignored)
• ./scripts/test-e2e-local.sh 09_graph_and_validation
• ./scripts/test-upgrade.sh (66 passed)

[waldemort-auto]

Validated against the diff at 68c2171: the claim holds. This fixes #327 at the root and legitimately supersedes #328.

Posting the detailed check since I raised the alternative that this PR correctly rejected.

The stack-depth objection was right, and this PR avoids the trap

My earlier suggestion on #328 — serde_json::Deserializer::disable_recursion_limit() — would have kept parsing recursive but unbounded. That is a real stack-overflow hazard, and the caveat I attached (bound the insert_nodes walk) was insufficient: the overflow would occur during parsing, before any walker ran. This PR does something structurally different and better.

Storing children as Box<RawValue> means from_str::<Durofut> parses exactly one level and children stay opaque. The detail that makes it safe rather than merely deferred: serde_json captures a RawValue via ignore_value (v1.0.151, src/de.rs:1102-1160), which is iterative — it uses a heap Vec (self.scratch) as an explicit stack, with no check_recursion! on that path. Parse-time call-stack usage is therefore O(1) regardless of nesting depth; the depth cost moves from the call stack to the heap. That is why durofut_deserializes_beyond_serde_recursion_limit passes at 129 levels without risk.

So the protection is not removed, it is upgraded from incidental to intentional. The 128 limit was never a designed guard — it was a serde artifact that silently corrupted instead of erroring. MAX_GRAPH_DEPTH = 256 replaces it, and per my analysis on #328 that constant was previously unreachable dead code: nothing deep enough to trip it could survive the parse first. It is now reachable and enforced. Remaining recursive walkers:

Walker	Bounded?
validate_recursive_inner	Self-bounding — errors at 257, small frames
insert_nodes (src/dsl.rs)	Guarded — validate_recursive runs earlier in df.start
collect_nodes (src/explain.rs)	Was unguarded; this PR adds validate_recursive before both traversals

Worst case is ~257 shallow frames against PostgreSQL's default 2 MB max_stack_depth. The df.explain() path was the real exposure, and closing it is the right call.

Worth keeping from #328: one thing

Durofut::ensure (src/types.rs:1288-1295) is untouched here. The catch-all still silently wraps any unparseable envelope as a SQL node, with no log:

match serde_json::from_str::<Durofut>(s) {
    Ok(d) if VALID_NODE_TYPES.contains(&d.node_type.as_str()) => d,
    _ => Durofut {
        node_type: "SQL".to_string(),
        query: Some(s.to_string()),
        ..Default::default()
    },
}

This PR removes the trigger (serde recursion) but not the class (silent corruption on an unparseable envelope). It also slightly widens that class: the new deserialize_raw_object rejects non-object children with "Durofut children must be JSON objects", and that error is swallowed by this same catch-all into a silent SQL wrap rather than surfacing. Worth carrying over #328's loud-failure behaviour on that arm — though not its looks_like_serialized_durofut prefix sniff, which depends on node_type being the first serialized key and so is coupled to serde field-declaration order. Keying off Err(_) directly is more robust. The rest of #328 is subsumed; the tests here are strictly stronger.

Credit where due: this PR already removes one of the sibling fallbacks I flagged — the PL/pgSQL df.ensure_durofut WHEN OTHERS handler. Three remain in the same class: for_each_config_child and transform_config_children (Err(_) => Ok(()), silently skipping embedded condition_node/extra_nodes children) and is_durofut (unwrap_or(false)).

Two review points

1. collect_nodes uses .expect("validated Durofut child must deserialize") while insert_nodes uses pgrx::error! for the identical situation. A Rust panic inside a pgrx extension is a worse outcome than a clean PostgreSQL ERROR, and it encodes an invariant that spans two separate traversals. Suggest aligning src/explain.rs to pgrx::error! (or returning the formatted error, since explain_expression already returns diagnostic strings).

2. Depth errors now surface at df.start()/df.explain() rather than at the offending composer call. That is a deviation from the "fail at the construction boundary" suggestion in df.seq silently emits its own serialized expression tree as SQL beyond ~100 chained steps #327. It looks like the right trade to me — nothing corrupt is persisted, and df.start() raises before node insertion, so the false-success mode from the original report is gone — but flagging it as a conscious call rather than an oversight.

Test coverage

Materially stronger than #328: 129 levels via df.seq, 129 via the ?>/!> PL/pgSQL path, exactly 256 accepted, 257 rejected with the specific message, all eight composers round-tripping opaque children, and a 200-step end-to-end run. The 399-node assertion is the clincher — 200 SQL + 199 THEN is exactly the full materialization that previously corrupted to 73/72 in the #327 repro, so it pins the fix to the original numeric fingerprint.

One gap: no deep-chain regression for df.join, df.race, df.loop, or df.as. The fix is structural so they benefit automatically, and test_all_composers_round_trip_opaque_children covers them shallowly — but a single deep JOIN/RACE chain would lock in that they share the corrected path.

Full CI green at 68c2171 (Clippy & Tests PG17, Docker Build & E2E, both package validations), mergeable_state=clean.

[pinodeca]

Thanks for the detailed follow-through. The remaining silent-corruption items you flagged are addressed in a narrow follow-up: #328.

Carried over from your review:

• Durofut::ensure() no longer swallows an unparseable envelope into a raw SQL node. It now fails loudly, keying off the parse Err plus a generic-JSON node_type probe — robust to serialized field order, so it avoids the looks_like_serialized_durofut prefix sniff you (rightly) called out. This also surfaces the new deserialize_raw_object "Durofut children must be JSON objects" rejection instead of silently wrapping it.
• Review point 1: collect_nodes() in df.explain() now raises pgrx::error! instead of .expect()-panicking, matching insert_nodes() in df.start().

Deliberately left as-is:

• is_durofut() (unwrap_or(false)) — returning false is the correct contract for a boolean predicate.
• The for_each_config_child / transform_config_children top-level Err(_) guards already fail loudly on malformed condition_node / extra_nodes; the top-level arm only skips a genuinely non-JSON query field.
• Review point 2 (depth errors surfacing at df.start()/df.explain()) — treated as the intended trade-off you described, not a bug.

Tests: loud-failure on a corrupt envelope via df.seq, envelope detection independent of field order, and a deep JOIN chain to lock in that JOIN/RACE share the corrected opaque-child path. I stopped short of deep RACE/loop/as chains and an E2E test — happy to add those if you'd like them pinned explicitly.