Repository navigation
fix: bind HITL resumes to the reviewed tool invocation - #146
Conversation
Checkpoint model responses and completed tool outcomes so approval resumes reuse the reviewed request without repeating earlier side effects. Validate invocation bindings and fail closed on mismatched or legacy continuations. Honor claimed decisions across session-grant and auto-mode changes, preserve execution limits, isolate concurrent resumes, and retain mismatch errors when approval cleanup fails. Document the continuation format and add regression coverage for streaming, nested approvals, provider changes, and replay safety. Validation: make check passed with 1050 tests. Refs #139.
AmitAvital1
left a comment
There was a problem hiding this comment.
Reviewed the full HITL continuation change against #139, including the approval lifecycle, checkpoint replay semantics, nested agents, sequential approvals, policy changes, concurrency, execution limits, and stream/non-stream paths.
I found one additional fail-closed edge case during review: after a rebuild removed the pending tool, replay could finish with an unconsumed binding and mark the approval approved even though the reviewed invocation never reached its gate. Fixed in c39c2dd by requiring every resume binding to be consumed before graph completion, with regression coverage for both resume modes and synchronized docs/ADR.
The final implementation keeps resume state request-local, checkpoints model/tool substeps without repeating side effects, validates the security-relevant invocation while excluding transient provider call IDs, rejects mismatched/legacy/unconsumed continuations, and preserves execution-limit accounting. I found no remaining blocking issues or unnecessary architectural duplication.
Validation: CI check and ui are green; 1052 tests are collected. Approving.
Checkpoint model responses and completed tool outcomes so approval resumes reuse the reviewed request without repeating earlier side effects. Validate invocation bindings and fail closed on mismatched or legacy continuations.
Honor claimed decisions across session-grant and auto-mode changes, preserve execution limits, isolate concurrent resumes, and retain mismatch errors when approval cleanup fails. Reject resumed runs whose approval binding is never consumed, including rebuilt graphs that remove or block the reviewed invocation. Document the continuation format and add regression coverage for streaming, nested approvals, provider changes, replay safety, and removed pending tools.
Validation: CI
checkanduipassed atc39c2dd; pytest collects 1052 tests.Refs #139.