Skip to content

fix(btw): record runner errors and refused work runs - #170

Merged
YUZHEthefool merged 1 commit into
fix/btw-work-cancel-on-abort-153from
fix/btw-work-failure-detection-154
Sep 14, 2026
Merged

YUZHEthefool merged 1 commit into
fix/btw-work-cancel-on-abort-153from
fix/btw-work-failure-detection-154

Conversation

@YUZHEthefool

@YUZHEthefool YUZHEthefool commented Sep 11, 2026

Copy link
Copy Markdown
Member

Summary

Work runs that errored, or never ran at all, reported completed. Expected a
local provider error, a third-party runner error, and a request refused by
session admission to be failed.

Related issue

Fixes #154

Root cause

The work loop read a single failure marker, and nothing set it on these paths:

  • run_agent turns an err response into a user-facing error result and then
    drains its generator normally, which the loop cannot tell apart from an answer.
  • ThirdPartyAgentSubStage records _third_party_runner_error and nothing read it.
  • AgentRequestSubStage returns before building an Agent when the provider is
    disabled or the session has AI off, so the executor yielded nothing and the
    generator ending looked like success.

Reproduction

Enable BTW and the work loop, then (a) drive the real run_agent with a runner
that returns err, (b) drive the real ThirdPartyAgentSubStage non-streaming
handler with a runner that raises, and (c) drive the real AgentRequestSubStage
with session AI disabled. All three used to read completed. Each is a
regression test.

Implementation notes

Write the failure marker where the failure happens and read both markers in the
loop. The marker keys move into btw/types.py, which imports only the standard
library, so the writers (agent stages, run_agent) and their only reader cannot
drift apart; third_party.THIRD_PARTY_RUNNER_ERROR_EXTRA_KEY still resolves, now
by import. mark_work_run_failed keeps chat runs on their existing error path.

An executor that produced nothing also no longer counts as completed: a generator
ending only proves the task ran when something actually ran. That is the
"do not equate normal generator termination with success" criterion, kept narrow
enough that queued work is unaffected — it is owned by the semaphore, not by
this generator.

Validation

.venv/Scripts/python.exe -m pytest tests/unit -q
  5219 passed, 6 skipped

.venv/Scripts/python.exe -m ruff format --check <touched files>   # clean
.venv/Scripts/python.exe -m ruff check <touched files>            # clean
.venv/Scripts/python.exe -m pyright --project pyrightconfig.quality.json <touched files>
  0 errors

All five new tests were run against the unmodified tree first and failed with
WorkSessionStatus.COMPLETED, so they are real regression tests rather than
descriptions of current behavior.

make check, make quality, and the coverage-gated --test-profile all run
were not run locally on this Windows checkout; this PR relies on CI for those,
as #168 did.

After #168 was rebased onto the updated master head, this branch was rebased onto
the new head as well and the whole stack was re-validated at its tip:
pytest tests/unit -q 5233 passed / 6 skipped and
pytest --test-profile blocking -q 5354 passed / 6 skipped / 1 deselected.
The count above was taken on this branch before that rebase.

Compatibility and risk

  • No route, schema, configuration, or dependency change. BTW stays off by default.
  • THIRD_PARTY_RUNNER_ERROR_EXTRA_KEY keeps its name and value; its definition
    moved from third_party.py to btw/types.py.
  • A work run that reaches no Agent is now failed instead of completed. That
    is the intended behavior change.

Checklist

  • The change is focused and does not include unrelated refactoring.
  • I added or updated a regression test, or explained why a test is not practical.
  • I ran the relevant formatting, lint, build, and test commands.
  • User-visible behavior updates both docs/zh/ and docs/en/ when needed. (No user-visible text changed.)
  • OpenAPI, generated client, docs/public/openapi.json, and tests change together when routes or schemas change. (No route or schema change.)
  • No secrets committed. Runtime Python deps update pyproject.toml, requirements.txt, and uv.lock together. (No dependency change.)
  • I did not restore legacy shims, Python <3.14 fallbacks, or upstream publish/docs URLs as fork artifacts.
  • Breaking API or behavior changes use ! and a BREAKING CHANGE: footer. (None; the status change is the fix.)
  • I will not merge this PR myself. Merge needs a human maintainer review plus a separate AI-assisted review (AI_POLICY.md).
  • AI use follows AI_POLICY.md. Keep exactly one author note below. Do not fabricate the other.

Agent note

Goal: resolve #154 with an explicit terminal outcome instead of
a bare generator end, keeping BTW off by default and preserving authorization and
request identity.

Paths touched: astrbot/core/agent/btw/types.py,
astrbot/core/agent/btw/work_loop.py, astrbot/core/astr_agent_run_util.py,
astrbot/core/pipeline/process_stage/method/agent_request.py,
.../agent_sub_stages/internal.py, .../agent_sub_stages/third_party.py, and
unit tests (test_btw_work_loop.py extended, test_agent_request_sub_stage.py
extended).

Checks run: the commands under Validation, plus the new tests against the
unmodified tree.

Residual risk: run_agent marks a work run failed on any err response, even if
a later step were to succeed. An err already produced a user-visible error, so
reporting the run as failed is the honest reading.

Tools used: Claude Code (Opus 5) with the repository's AGENTS.md and AI_POLICY.md.

This PR is stacked on #169; merge that first.

The work loop read a single failure marker, so three failing paths ended as
completed:

- run_agent reports a provider error as a user-facing result and then drains
  its generator normally, which is indistinguishable from an answer.
- The third-party stage records _third_party_runner_error and nothing read it.
- A run the session refuses on admission never reaches an Agent, so its
  executor yields nothing and the generator ending looked like success.

Write the failure marker from each of those paths and read both markers in the
work loop.  The marker keys move into btw/types.py so the writers and their
only reader cannot drift apart; mark_work_run_failed keeps chat runs on their
existing error path.

An executor that produced nothing also no longer counts as completed: a
generator ending only proves the task ran when something actually ran.

Fixes #154
AI-Generated: true
Generated-At: 2026-09-11T13:48:32Z
@YUZHEthefool
YUZHEthefool added this pull request to stack #174 September 11, 2026 15:57
@YUZHEthefool
YUZHEthefool force-pushed the fix/btw-work-failure-detection-154 branch from 3a0463e to bec2549 Compare September 11, 2026 16:00
@YUZHEthefool
YUZHEthefool merged commit 393fd93 into master Sep 14, 2026
25 checks passed
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.

[bug] BTW 模型与第三方执行器返回错误后工作状态仍显示已完成

1 participant