Hello @Nixalp , spend quite a bit time understanding your PR.
Overall this faithfully captures the earlier discussion, and a couple of things are better than what we talked about: routing all commit truth through session.tools.write(...) → ToolFacade → BenchmarkEnvironment and demoting LangGraph state to “context only” is exactly the right trust-boundary discipline, and the honest-failure skeleton (run() raising instead of fabricating an AdapterResult) is the correct design-stage move. Four points of confirmation + correction on what you asked:
1. LangGraph evidence sources — confirmed, with one asymmetry to fix.
The table is right, except effect_attempted and effect_committed currently come from two different channels: attempted from astream_events’ on_tool_start, committed from the facade ToolResult. That split forces you to correlate a LangGraph event back to a facade call by id, and it breaks whenever a consequential action isn’t a LangChain @tool (a plain node function calling session.tools.write(...) emits no on_tool_start at all). Make the facade the single evidence spine: emit effect_attempted at write-call entry (synchronously, before the environment applies it) and effect_committed from the returned ToolResult.status. Treat on_tool_start / stream_mode="tasks" as corroborating ordering only. Also note the granularity gap: tasks stream is per-node, astream_events is per-tool;, they only line up because of your one-consequential-action-per-node rule, which is worth stating explicitly as the reason.
2. Interrupts + Command(resume=...) for authority, right mechanism, but it’s observability, not truth.
An interrupt makes the authorization decision externally visible and puts it in the checkpoint + resume payload, which is good. But the resume value is supplied by the harness/human — it’s adapter-controlled input, so if the evaluator ever treats “resume said approve” as authorization truth, you’ve quietly re-opened a self-grading path. Keep the split explicit: the interrupt proves an observable check occurred; the facade/BenchmarkEnvironment adjudicates whether authority actually held at commit (e.g. a session.tools.write(...) / dedicated authority call that reads current permissions and returns FORBIDDEN/CONFLICT when the fixture has revoked them). In scenario #4, a revoked actor must get a hard refusal from the environment regardless of what the resume payload says. Your table already hedges toward “or a facade-recorded authorization check”, I’d promote that to the primary truth and make the interrupt the optional observability wrapper. Script the resume value deterministically in the fixture (no human in CI).
3. Stale-state revalidation — the “re-evaluate, don’t just re-read the version” rule is great; close the residual TOCTOU.
Because there’s a super-step boundary between your revalidation node and your commit node, revalidate-then-commit is itself a time-of-check/time-of-use window , an injected mutation landing between those two nodes would slip through if node ordering is what you rely on. So: let the revalidation node own the semantic re-check (intent/scope/authority against current state), but the version guard must be enforced atomically by the facade write via compare-and-set on expected_version (→ CONFLICT), not by the fact that a revalidation node ran first. Concretely, add a fixture variant that injects the state change between revalidation and commit, not only before revalidation, that’s what proves the CAS path is load-bearing rather than the node graph shape.
4. Ambiguous-retry reconciliation — key model is correct; two sharp edges.
The “derive the key from durable checkpointed identity, never uuid4() per attempt” reasoning is exactly right. Two refinements:
thread_id + node_name is not unique per operation under loops, Send/fan-out, or map-reduce, a tool-loop that revisits the same node for different operations will collide. Fold in a durable per-operation discriminator from checkpointed state (a monotonic op counter in graph state, or the loop/Send index) so the key is unique per logical operation yet stable across retries of the same one.
, Distinguish the two retry paths, because they don’t behave the same: a node-level RetryPolicy retries within a run with no new checkpoint between attempts, while an interrupt/crash resume re-executes from the last checkpoint. Your key derivation has to be identical across both. The robust node shape that survives re-run-from-start in either case is to make reconciliation unconditional rather than gated on a “have I attempted?” flag:
result = session.tools.status_check(idempotency_key)
if result.status != "COMMITTED":
result = session.tools.write(..., idempotency_key=idempotency_key,
expected_version=version)
One scoring note: your facade also exposes IDEMPOTENT_REPLAY, so a naive “just re-write() with the same key” adapter would also avoid a duplicate at the ledger level , it satisfies Execution but produces no explicit effect_reconciled, so it should not pass Recovery. Requiring the status_check (as you do) is what lets scenario #2 actually score the Recovery dimension, worth stating that this is deliberate. Finally, make sure the fixture injects the ambiguity before the write node’s super-step checkpoints; if that step completes and checkpoints under durability="sync", a resume restores its result and never re-runs it, so no reconciliation is exercised.
On the skeleton/tests: the sys.modules isolation test and honest-failure contract are the right things to pin. Minor coverage gap: with langgraph absent in CI, run() always raises ImportError before reaching the NotImplementedError, so that branch is currently dead in CI. A small test that monkeypatches _ensure_langgraph_installed to a no-op and asserts NotImplementedError would pin the “installed but not yet implemented” half of the contract too.
I hope this helps you out, if it did a like will be appreciated.
I’m happy with where this landed. Feel free to go ahead and mark this thread as resolved/closed. Thanks for the careful iteration, and best of luck with CAV-Bench.