refactor: extract finalize_run helper to deduplicate Runner exit paths#56
Merged
scmmishra merged 1 commit intochatwoot:mainfrom Mar 13, 2026
Conversation
The Runner#run method had five near-identical blocks that each built a RunResult, emitted agent_complete and run_complete callbacks, and returned. This copy-paste pattern existed across: - Handoff validation error (AgentNotFoundError) - Non-handoff halt response - Normal completion (no tool calls) - MaxTurnsExceeded rescue - StandardError rescue Extract a single `finalize_run` private method that centralises the save-state → build-result → emit-callbacks → return sequence. Also remove the unnecessary `result` → `response` variable alias introduced by the intermediate assignment (`result = if ...; response = result`), assigning directly to `response` instead.
✅ Deploy Preview for ruby-ai-agents canceled.
|
Member
|
@codex please review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
scmmishra
approved these changes
Mar 13, 2026
sergiobayona
added a commit
to sergiobayona/ai-agents
that referenced
this pull request
Mar 17, 2026
…run refactor Main branch introduced a `finalize_run` helper (chatwoot#56) that centralised the exit path. Our guardrails branch had inline output guards + manual finalization. This commit resolves the conflict and re-applies all three guard fixes on top of the new architecture. Changes re-applied: 1. GuardRunner.run now tracks `any_rewrite` across the chain and returns `action: :rewrite` when any guard rewrote content. Previously always returned `:pass`, making caller `.rewrite?` checks dead code. 2. `last_message_matches?` moved below the input guard block so dedup compares against post-guard (potentially rewritten) input. Prevents stale dedup from silently discarding guard rewrites. 3. Output guards serialize Hash/Array content (from response_schema) to JSON before the guard chain and deserialize after rewrite, so guards always operate on Strings. Conflict resolution: output guards run before `finalize_run`, passing the guarded `final_output` to the helper. Tripwire rescue remains separate (needs `guardrail_tripwire` field not supported by `finalize_run`). Tests: 10 new examples covering input guard rewrites, output guard rewrites, structured output guards (redact/tripwire/pass-through), and dedup regression. 432 examples, 0 failures, 98.24% line coverage.
sergiobayona
added a commit
to sergiobayona/ai-agents
that referenced
this pull request
Mar 18, 2026
…run refactor Main branch introduced a `finalize_run` helper (chatwoot#56) that centralised the exit path. Our guardrails branch had inline output guards + manual finalization. This commit resolves the conflict and re-applies all three guard fixes on top of the new architecture. Changes re-applied: 1. GuardRunner.run now tracks `any_rewrite` across the chain and returns `action: :rewrite` when any guard rewrote content. Previously always returned `:pass`, making caller `.rewrite?` checks dead code. 2. `last_message_matches?` moved below the input guard block so dedup compares against post-guard (potentially rewritten) input. Prevents stale dedup from silently discarding guard rewrites. 3. Output guards serialize Hash/Array content (from response_schema) to JSON before the guard chain and deserialize after rewrite, so guards always operate on Strings. Conflict resolution: output guards run before `finalize_run`, passing the guarded `final_output` to the helper. Tripwire rescue remains separate (needs `guardrail_tripwire` field not supported by `finalize_run`). Tests: 10 new examples covering input guard rewrites, output guard rewrites, structured output guards (redact/tripwire/pass-through), and dedup regression. 432 examples, 0 failures, 98.24% line coverage. refactor: consolidate tripwire rescue into finalize_run for rescue ordering safety The Tripwire rescue had 15 lines of inline finalization duplicating what finalize_run already does. If anyone reordered the rescue clauses, Guard::Tripwire (a StandardError subclass) would fall into the generic StandardError rescue, silently losing guardrail_tripwire metadata. Changes: - Add `guardrail_tripwire:` kwarg to finalize_run so it can build tripwire-aware RunResults - Collapse Tripwire rescue to a single finalize_run call - Add `raise if e.is_a?(Guard::Tripwire)` safety net in the StandardError rescue to prevent silent metadata loss Tests: 2 new examples verifying tripwire RunResult metadata and callback emission through the finalize_run path. 434 examples, 0 failures, 98.24% line coverage.
sergiobayona
added a commit
to sergiobayona/ai-agents
that referenced
this pull request
Mar 18, 2026
…run refactor Main branch introduced a `finalize_run` helper (chatwoot#56) that centralised the exit path. Our guardrails branch had inline output guards + manual finalization. This commit resolves the conflict and re-applies all three guard fixes on top of the new architecture. Changes re-applied: 1. GuardRunner.run now tracks `any_rewrite` across the chain and returns `action: :rewrite` when any guard rewrote content. Previously always returned `:pass`, making caller `.rewrite?` checks dead code. 2. `last_message_matches?` moved below the input guard block so dedup compares against post-guard (potentially rewritten) input. Prevents stale dedup from silently discarding guard rewrites. 3. Output guards serialize Hash/Array content (from response_schema) to JSON before the guard chain and deserialize after rewrite, so guards always operate on Strings. Conflict resolution: output guards run before `finalize_run`, passing the guarded `final_output` to the helper. Tripwire rescue remains separate (needs `guardrail_tripwire` field not supported by `finalize_run`). Tests: 10 new examples covering input guard rewrites, output guard rewrites, structured output guards (redact/tripwire/pass-through), and dedup regression. 432 examples, 0 failures, 98.24% line coverage. refactor: consolidate tripwire rescue into finalize_run for rescue ordering safety The Tripwire rescue had 15 lines of inline finalization duplicating what finalize_run already does. If anyone reordered the rescue clauses, Guard::Tripwire (a StandardError subclass) would fall into the generic StandardError rescue, silently losing guardrail_tripwire metadata. Changes: - Add `guardrail_tripwire:` kwarg to finalize_run so it can build tripwire-aware RunResults - Collapse Tripwire rescue to a single finalize_run call - Add `raise if e.is_a?(Guard::Tripwire)` safety net in the StandardError rescue to prevent silent metadata loss Tests: 2 new examples verifying tripwire RunResult metadata and callback emission through the finalize_run path. 434 examples, 0 failures, 98.24% line coverage.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Runner#run method had five near-identical blocks that each built a RunResult, emitted agent_complete and run_complete callbacks, and returned. This copy-paste pattern existed across:
Extract a single
finalize_runprivate method that centralises the save-state → build-result → emit-callbacks → return sequence.Also remove the unnecessary
result→responsevariable alias introduced by the intermediate assignment (result = if ...; response = result), assigning directly toresponseinstead.