fix(agent): keep guardrail retries on the async path in kickoff_async - #7292
fix(agent): keep guardrail retries on the async path in kickoff_async#7292bunnysayzz wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe agent now uses dedicated asynchronous finalization and guardrail retry paths for ChangesAsync guardrail retry
Sequence Diagram(s)sequenceDiagram
participant Agent
participant Guardrail as _process_kickoff_guardrail_async
participant Executor as _execute_and_build_output_async
Agent->>Guardrail: evaluate kickoff output
Guardrail->>Executor: retry rejected output asynchronously
Executor-->>Guardrail: return retried output
Guardrail->>Guardrail: evaluate guardrail again
Guardrail-->>Agent: return accepted output or validation error
Merge Risk: ⚪ Minimal · up to Async guardrail retries now await the asynchronous execution path, allowing later valid outputs to succeed and retry exhaustion to raise the expected validation error. No current merge-blocking risk is identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
6a375e6 to
4332800
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
4332800 to
c760d9e
Compare
c760d9e to
c78bcc4
Compare
|
Rebased onto latest main, no conflicts. The async guardrail tests (24 across the async kickoff and task guardrail suites) all pass on the rebased head. |
Agent.kickoff_async()with a guardrail routes retries through the syncexecutor.invoke(). Under a running event loop that hands back an unawaited coroutine instead of a result dict, and_build_output_from_resultcrashes onresult.get("output", "").Fix: async variants
_finalize_kickoff_async/_process_kickoff_guardrail_asyncthat re-execute via_execute_and_build_output_async. To avoid duplicating the whole guardrail flow, the guardrail resolution and result-folding steps are extracted into shared helpers (_resolve_guardrail_callable,_apply_guardrail_result) used by both the sync and async paths; sync behavior is unchanged.Tests: new
test_agent_kickoff_guardrail_async.py— a fail-once guardrail overkickoff_asyncwith the sync execute path rigged to explode proves the retry stays async (fails pre-fix), plus retry-exhaustion still raising. Existing sync guardrail tests (6) still pass.Fixes #7252 (I was assigned this issue; opening alongside #7289 so maintainers can pick).