Skip to content

fix(core): Graceful handling of NFE loops and incremental SARIF flushing#805

Open
Tanmay9223 wants to merge 2 commits into
usestrix:mainfrom
Tanmay9223:fix-797-nfe-loop-crash
Open

fix(core): Graceful handling of NFE loops and incremental SARIF flushing#805
Tanmay9223 wants to merge 2 commits into
usestrix:mainfrom
Tanmay9223:fix-797-nfe-loop-crash

Conversation

@Tanmay9223

Copy link
Copy Markdown

🎯 What: Catch MaxTurnsExceeded unhandled exception to prevent scan crashes and flush SARIF incrementally.
💡 Why: When child agents get stuck in Non-Lifecycle Final Output (NFE) loops in non-interactive scans, they raise a MaxTurnsExceeded exception. Previously, this exception went unhandled in the _child_loop task, crashing the root run_strix_scan process without saving the SARIF report for agents that already successfully completed their work.
Verification: Verified locally that Strix system tests pass without regressions, and verified via test execution that MaxTurnsExceeded is gracefully handled with incremental SARIF flushing on agent completion.
Result: Scans no longer hard-crash on NFE loops. Findings are safely persisted, ensuring partial scan success is not lost. Fixes #797.

When child agents get stuck in Non-Lifecycle Final Output (NFE)
loops in non-interactive scans, they raise a MaxTurnsExceeded
exception. Previously, this exception went unhandled in the child
loop task, crashing the root scan process without saving the
SARIF report for agents that had already completed their work.

This change gracefully handles MaxTurnsExceeded and flushes:
- strix/core/execution.py: Catch MaxTurnsExceeded in the child loop.
- strix/core/runner.py: Gracefully stop scan on root MaxTurnsExceeded.
- strix/tools/agents_graph/tools.py: Save run data in agent_finish.

The SARIF report is now saved incrementally, ensuring partial results
are preserved even if a subset of agents fail to complete properly.
@Tanmay9223
Tanmay9223 force-pushed the fix-797-nfe-loop-crash branch from feb3eb9 to 744d28c Compare July 19, 2026 13:09
@greptile-apps

greptile-apps Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR addresses two related problems: NFE (Non-Lifecycle Final Output) loops that could cause child agents to spin indefinitely, and unhandled MaxTurnsExceeded exceptions that previously crashed the root scan without persisting already-completed findings.

  • NFE limit cap: invalid_final_output_limit is now capped at min(5, max(1, max_turns)), bounding retry loops regardless of the configured max_turns value.
  • Exception handling: MaxTurnsExceeded is caught in both _child_loop and at the top-level run_strix_scan; a broad except Exception clause is also added to _child_loop as a safety net.
  • Incremental SARIF flushing: save_run_data() is called inside agent_finish immediately after each child agent completes, persisting findings before the overall scan concludes.

Confidence Score: 3/5

Safe to merge for the NFE cap and SARIF flushing, but the child-loop exception handlers do not update the coordinator status in cases where the exception arrives without a prior status update, which could leave child agents permanently stuck in running and stall the root scan.

The new exception clauses in _child_loop handle the primary NFE-triggered MaxTurnsExceeded correctly (status is already set to crashed before the raise), but SDK-level MaxTurnsExceeded and any exception caught by the broad except Exception block will leave the child in running state with no parent notification. If anything routes through those paths the root scan may hang indefinitely waiting for children to settle.

strix/core/execution.py — specifically the new exception handlers in _child_loop that lack a coordinator status update fallback.

Important Files Changed

Filename Overview
strix/core/execution.py Caps the NFE retry limit at 5 and adds exception handling in _child_loop, but the new except MaxTurnsExceeded and except Exception clauses don't update child agent status in the coordinator when the exception reaches there without a prior status update.
strix/core/runner.py Adds top-level MaxTurnsExceeded handling in run_strix_scan with proper cleanup; import ordering places a third-party import inside strix.core import blocks.
strix/tools/agents_graph/tools.py Adds incremental SARIF flushing via save_run_data() on agent_finish; uses a deferred import to access get_global_report_state which hides a potential circular import.
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
strix/core/execution.py:584-587
**Missing child status update on caught exceptions**

When `MaxTurnsExceeded` is caught here from the SDK-level runner (not the NFE limit path), the child agent's coordinator status is never updated from `"running"` and `_notify_parent_on_crash` is never called. The NFE-triggered `MaxTurnsExceeded` is safe because `set_status(agent_id, "crashed")` is called in `_run_noninteractive_until_lifecycle` before raising — but a genuine SDK max-turns breach propagates through `_run_cycle`'s `if not interactive: raise` path with no prior status update, leaving the child permanently stuck in `"running"` in the coordinator. The same gap affects the broad `except Exception` block: any exception that escapes `_run_cycle` without first setting a terminal status will leave the child as `"running"` with no parent notification, potentially causing the root scan to hang indefinitely waiting for all children to settle.

### Issue 2 of 3
strix/core/runner.py:32-34
The `from agents.exceptions import MaxTurnsExceeded` import is placed inside the `strix.core.*` import block, breaking the import grouping convention used throughout the file. It should sit with other third-party library imports.

```suggestion
from agents.exceptions import MaxTurnsExceeded
from strix.core.hooks import BudgetExceededError, ReportUsageHooks
from strix.core.inputs import (
```

### Issue 3 of 3
strix/tools/agents_graph/tools.py:577-579
**Deferred import suggests unresolved circular dependency**

`strix.report.state` is not imported at the top of `tools.py` — it only appears as a lazy import inside `agent_finish`. This pattern is commonly used to break circular imports but is inconsistent with the rest of the file's import style and hides the dependency from static analysis tools. It would be worth confirming whether a top-level import causes a circular import, and if so, addressing the root cause (e.g., extracting the shared function to a separate utility module) rather than papering over it with a deferred import.

Reviews (1): Last reviewed commit: "fix(core): handle MaxTurnsExceeded and f..." | Re-trigger Greptile

Comment thread strix/core/execution.py
Comment thread strix/core/runner.py
Comment thread strix/tools/agents_graph/tools.py Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

deep mode: non-lifecycle final output loop causes MaxTurnsExceeded, 29 findings lost

1 participant