Skip to content

fix(agent): surface background result runs ending in ERROR state - #10579

Open
he-yufeng wants to merge 1 commit into
AstrBotDevs:masterfrom
he-yufeng:fix/background-result-agent-error-state
Open

he-yufeng wants to merge 1 commit into
AstrBotDevs:masterfrom
he-yufeng:fix/background-result-agent-error-state

Conversation

@he-yufeng

@he-yufeng he-yufeng commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Sibling of #9980. When a background task finishes, _wake_main_agent_for_background_result builds a main agent to process and deliver the result. That path runs the same step_until_done loop as the cron wakeup, but never checks runner.state afterwards.

A run that fails inside the runner (e.g. a malformed function call at max steps, or every provider failing) raises no exception. The cron path got a guard in #9987 that raises so the job is recorded as failed; the background twin still falls through and:

  • persists a normal [BackgroundTask] ... finished history note, appending the runner's error text as if it were a result ("I finished the task, here is the result: "), and
  • surfaces nothing about the failure anywhere.

Fix

Mirror the cron guard: after the loop, if runner.state == AgentState.ERROR, raise RuntimeError with the completion text as detail. The raise propagates to the _run_in_background / _run_handoff_in_background wrappers, which already log the failure with the task id, instead of recording a misleading success in history.

Tests

  • New regression test: an ERROR-state result run raises and persist_agent_history is not called.
  • test_proactive_agent_uses_session_fallback_models pinned the old asymmetry (pytest.raises only for the cron entrypoint even though it parametrizes both entrypoints); extended the expectation to both paths via the shared message substring.
  • Existing runner doubles in test_astr_agent_tool_exec.py now declare state = AgentState.DONE to match the real runner interface.

Local: pytest tests/unit 1527 passed, ruff format/check clean.

Summary by Sourcery

Propagate failures from background result agent runs so errored tasks are reported instead of presented as successful completions.

Bug Fixes:

  • Surface background agent result runs that end in an ERROR state by raising a runtime error instead of recording misleading successful completion history.

Tests:

  • Add regression coverage confirming ERROR-state background runs raise and do not persist agent history.
  • Update agent entrypoint expectations and runner test doubles to reflect shared error-state handling and runner state.

The background wakeup path runs the same step_until_done loop as the
cron path but never checked runner.state, so a run that failed inside
the runner (e.g. malformed function call at max steps) was persisted
as a normal "finished" history note and the error never surfaced.
Mirror the cron guard from AstrBotDevs#9980: raise with the completion detail so
the failure is logged by the background task wrapper instead of being
recorded as a success.

The failover test pinned the old asymmetry (only cron raised); extend
the expectation to both wakeup entrypoints and add a regression test
that an ERROR-state result run raises and skips persist_agent_history.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="astrbot/core/astr_agent_tool_exec.py" line_range="658" />
<code_context>
             # agent will send message to user via using tools
             pass
         llm_resp = runner.get_final_llm_resp()
+        if runner.state == AgentState.ERROR:
+            # The result run failed (e.g. malformed function call at max steps)
+            # but no exception escapes the runner; without this the failure was
</code_context>
<issue_to_address>
**Delivered messages disappear from history**

When the background agent sends a user-facing message during one step and enters ERROR on a later step, `_wake_main_agent_for_background_result` raises before `persist_agent_history` runs, so the already-delivered message is omitted from conversation history and later turns can repeat or contradict it.

Persist the delivered message before raising for the background agent's ERROR state.
</issue_to_address>

Sourcery assessment

Approval pending. 1 finding to address first.

Blocking findings: astrbot/core/astr_agent_tool_exec.py:658


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

# agent will send message to user via using tools
pass
llm_resp = runner.get_final_llm_resp()
if runner.state == AgentState.ERROR:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium · Delivered messages disappear from history

When the background agent sends a user-facing message during one step and enters ERROR on a later step, _wake_main_agent_for_background_result raises before persist_agent_history runs, so the already-delivered message is omitted from conversation history and later turns can repeat or contradict it.

Persist the delivered message before raising for the background agent's ERROR state.

Prompt for AI agents
In `astrbot/core/astr_agent_tool_exec.py` at line 658:

**Delivered messages disappear from history**

When the background agent sends a user-facing message during one step and enters ERROR on a later step, `_wake_main_agent_for_background_result` raises before `persist_agent_history` runs, so the already-delivered message is omitted from conversation history and later turns can repeat or contradict it.

Persist the delivered message before raising for the background agent's ERROR state.

@he-yufeng

Copy link
Copy Markdown
Contributor Author

On the sourcery blocking finding (delivered messages allegedly disappearing from history): I checked the persistence path, and the mechanism it assumes does not exist.

SendMessageToUserTool.call sends through the event context and never writes to conversation history; the text it delivers is not persisted by that tool on any path, success or failure. persist_agent_history appends exactly two synthetic entries (the "output your last task result" marker and the [BackgroundTask] ... finished summary note built from the task meta and the final llm response). It never contains the delivered message, so the ERROR raise cannot drop delivered text from history: that text was never written by this function.

What the raise does skip on failure is the synthetic "finished" note, and that is the point of this PR. Before it, a failed result run was persisted as a normal finished note with the runner's error text appended as if it were the result ("I finished the task, here is the result: "), which is the exact shape #9980 fixed on the cron path. The failure now surfaces as an exception from the wake instead of being written into the user's conversation as a fake success.

If the project would rather record failures inside conversation history too, that is a product decision and a small follow-up; I kept the #9987 semantics on purpose so both wake paths agree.

This branch has not been deployed

No deployments
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.

1 participant