fix(core): Graceful handling of NFE loops and incremental SARIF flushing#805
fix(core): Graceful handling of NFE loops and incremental SARIF flushing#805Tanmay9223 wants to merge 2 commits into
Conversation
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.
feb3eb9 to
744d28c
Compare
Greptile SummaryThis PR addresses two related problems: NFE (Non-Lifecycle Final Output) loops that could cause child agents to spin indefinitely, and unhandled
Confidence Score: 3/5Safe 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 strix/core/execution.py — specifically the new exception handlers in Important Files Changed
Prompt To Fix All With AIFix 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 |
🎯 What: Catch
MaxTurnsExceededunhandled 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
MaxTurnsExceededexception. Previously, this exception went unhandled in the_child_looptask, crashing the rootrun_strix_scanprocess 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
MaxTurnsExceededis 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.