fix(flows): circuit-breaker counts validate/dry_run body-level ok:false failures

[graycyrus]

Summary

RepeatedToolFailureMiddleware — the loop breaker that halts an agent run when a tool call keeps failing with no progress (issue #4249) — only treated a tool result as a "failure" when result.error.is_some(). validate_workflow and dry_run_workflow report an invalid graph or aborted sandbox run via ToolResult::success (result.error == None) with a JSON body carrying a top-level "ok": false instead. The breaker never saw those as failures, so when workflow_builder loops calling validate_workflow/dry_run_workflow on a graph it can't fix, it burns the full extended 50-iteration budget and only stops at the 600s FLOW_BUILD_TIMEOUT_SECS wall clock — with no useful root-cause summary for the user.

Problem

• src/openhuman/tinyagents/middleware.rs (RepeatedToolFailureMiddleware::after_tool, ~line 2315): the no-progress/repeated-failure ladder (NoProgressTracker) reads attempt.error: Option<&str> as its sole success/failure signal — None clears every counter as "progress was made."
• src/openhuman/flows/builder_tools.rs:
  • validate_workflow (~768-777) returns ToolResult::success with "ok": false in the body for a structurally invalid graph or a failed builder gate.
  • dry_run_workflow (~2846-2867, ~3059) returns ToolResult::success with "ok": false for a sandbox abort/error or a dry run with null-resolved args / node errors.

Since result.error stays None in both cases, the breaker's ToolAttempt.error field is None, which the crate tracker (NoProgressTracker::record) treats as a success — it never trips the identical-repeat nudge/halt ladder, and the run only ends on the outer 600s timeout with a generic cap error instead of a root-cause halt summary.

Solution

Added fn is_body_level_failure(name: &str, content: &str) -> bool (src/openhuman/tinyagents/middleware.rs) that, only for validate_workflow and dry_run_workflow, parses content as JSON and returns true when the top-level "ok" key is false. It's intentionally scoped to those two tool names — a generic "ok": false from some other tool may be legitimate data, not a failure signal — and tolerant of non-JSON/missing-ok content (returns false rather than guessing).

In after_tool:

• Compute body_level_failure once, gated on result.error.is_none() so a result that already carries a real error is never double-counted through the body check.
• Extend the model-facing failure_text to include the body content when body_level_failure is true (previously empty for a None-error result), so it's available to the same halt-summary text the crate ladder already produces.
• Union the signal into the ToolAttempt.error fed to NoProgressTracker::record: Some(err) when result.error is set (unchanged), Some(body content) when only the body-level signal fired, None otherwise (true success). This feeds the exact same nudge (at repeat 2) / halt (at repeat 3, or the varied-failure backstop at 6 consecutive) ladder as a real tool error, with no changes to hard_reject, the recoverable-failure headroom, or the terminal-inference fast-halt — those all remain gated on result.error.is_some() as before, since a body-level validation failure is deterministic, not a transient/recoverable one.

Testing

Added to the RepeatedToolFailureMiddleware test module in src/openhuman/tinyagents/middleware.rs:

• is_body_level_failure_detects_validate_and_dry_run_only — unit-tests the predicate directly: ok:false detected for both tool names, ok:true never counts, an unrelated tool's ok:false is ignored, and non-JSON/missing-ok content is tolerated (returns false).
• repeated_validate_workflow_ok_false_trips_the_breaker — repeated validate_workflow ok:false results (real ToolResult::success, error: None) on the same graph trip the breaker (previously never would).
• single_or_unrelated_ok_false_does_not_falsely_trip_the_breaker — a single validate_workflow ok:false doesn't halt, and a repeated unrelated tool's ok:false is never reinterpreted as a failure.
• existing_error_is_some_behavior_is_unchanged_by_body_level_check — the pre-existing result.error.is_some() path (three identical errors → halt) is unchanged, and a result with both error set and body-level ok:false is counted once per call, not double-counted.

GGML_NATIVE=OFF cargo check --manifest-path Cargo.toml            # clean
GGML_NATIVE=OFF cargo test --lib openhuman::tinyagents::middleware::   # 49 passed
GGML_NATIVE=OFF cargo test --lib openhuman::flows::                    # 383 passed
cargo fmt --manifest-path Cargo.toml -- --check                        # clean

Acceptance

• GGML_NATIVE=OFF cargo check passes clean
• cargo test --lib openhuman::tinyagents::middleware:: passes (49/49, including the 4 new tests)
• cargo test --lib openhuman::flows:: passes (383/383, unaffected)
• cargo fmt applied, --check clean
• Only validate_workflow / dry_run_workflow body-level ok:false is reinterpreted as a failure; every other tool's behavior (including a generic ok:false elsewhere) is unchanged
• result.error.is_some() behavior (hard-reject, recoverable-failure headroom, terminal-inference fast-halt, identical/varied nudge-halt ladder) is unchanged
• A result with both error set and body-level ok:false is not double-counted

Summary by CodeRabbit

• Bug Fixes
  • Improved handling of workflow validation and dry-run failures reported in tool results.
  • Repeated workflow failures now trigger the appropriate retry guidance and halt behavior.
  • Prevented duplicate failure counting when a tool result includes both an error and a failed status.
  • Results from unrelated tools with a failed status continue to follow existing behavior.

[coderabbitai]

📝 Walkthrough

Walkthrough

RepeatedToolFailureMiddleware now recognizes top-level ok: false responses from workflow validation tools as failures, feeds them into no-progress tracking, avoids double-counting existing errors, and adds regression tests.

Changes

Workflow failure tracking

Layer / File(s)	Summary
Failure detection and classification src/openhuman/tinyagents/middleware.rs	Detects body-level failures for validate_workflow and dry_run_workflow, then derives failure text when no explicit error exists.
No-progress integration and regression tests src/openhuman/tinyagents/middleware.rs	Feeds synthesized failures into NoProgressTracker and tests targeted tools, unrelated tools, repeated failures, and combined error cases.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• tinyhumansai/openhuman#4480: Modifies the same middleware’s no-progress escalation behavior.

Suggested labels: bug

Suggested reviewers: senamakel, m3ga-mind, yellowsnnowmann

Poem

A bunny spots ok: false in the hay,
Counts each workflow stumble away.
No double-counting in sight,
The breaker trips just right—
And tests keep the burrow safe today.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: circuit-breaker handling for validate/dry_run body-level ok:false failures.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/openhuman/tinyagents/middleware.rs`:
- Around line 2441-2447: Add a tracing::debug! event in the body_level_failure
detection branch, logging a grep-friendly message and result.name while
excluding result.content. Place it before the existing nudge/halt handling so
the ok:false transition is recorded when is_body_level_failure returns true.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c63148ec-9ed7-4049-95f3-5280d491207c

📥 Commits

Reviewing files that changed from the base of the PR and between 095e3c3 and b0e677a.

📒 Files selected for processing (1)

• src/openhuman/tinyagents/middleware.rs

[coderabbitai]

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Log body-level failure classification.

Emit a tracing::debug! event when this branch detects ok: false, including the tool name but not the response body. This makes the new failure-state transition diagnosable before nudge/halt.

As per coding guidelines, “New or changed flows must include verbose, grep-friendly diagnostics for … branches … state transitions.”

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/openhuman/tinyagents/middleware.rs` around lines 2441 - 2447, Add a
tracing::debug! event in the body_level_failure detection branch, logging a
grep-friendly message and result.name while excluding result.content. Place it
before the existing nudge/halt handling so the ok:false transition is recorded
when is_body_level_failure returns true.

Source: Coding guidelines

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b0e677ac3d

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Detect ok:false before tool-output rewriting

In the session harness I checked, context_mw.install is pushed after the repeated-failure breaker and after_tool runs in reverse order, so ToolOutputMiddleware rewrites the result before this parse runs. For chat turns with the default 16 KiB tool-result budget/artifact preview, or when TokenJuice compacts these non-exempt tools, a large validate_workflow/dry_run_workflow {"ok": false} body is replaced with a preview/truncated non-JSON string; serde_json::from_str then returns false and the loop still is not counted as a failure. Capture the raw ok:false signal before output rewriting, or make these tool outputs exempt/parse from the persisted raw body, otherwise larger invalid graphs still burn the iteration budget.

Useful? React with 👍 / 👎.