Skip to content

fix(btw): record an aborted work run as cancelled - #169

Merged
YUZHEthefool merged 1 commit into
fix/btw-dual-loop-history-152from
fix/btw-work-cancel-on-abort-153
Sep 14, 2026
Merged

YUZHEthefool merged 1 commit into
fix/btw-dual-loop-history-152from
fix/btw-work-cancel-on-abort-153

Conversation

@YUZHEthefool

@YUZHEthefool YUZHEthefool commented Sep 11, 2026

Copy link
Copy Markdown
Member

Summary

A work task the user stopped reported completed. Expected /work status to
report cancelled; actual it reported completed after the executor reported
its abort.

Related issue

Fixes #153

Root cause

run_agent reports an abort by setting agent_user_aborted and clearing
agent_stop_requested, because an aborted run must not look like a stop request
that is still pending. WorkLoop._execute read only the cleared flag, so the
abort fell through to completed.

Reproduction

Enable BTW and the work loop with a local Agent, start /work <task>, send
/task stop while it runs, wait for the executor to report aborted, then read
/work status. The regression test drives that same control flow: the real
run_agent is the work loop's executor and only the model runner is a double
configured to emit aborted.

Implementation notes

Treat either marker as a cancellation. The test asserts agent_user_aborted is True and agent_stop_requested is False before asserting the status, so it
fails again if the loop ever goes back to depending on the stop flag alone.

Validation

.venv/Scripts/python.exe -m pytest tests/unit -q
  5213 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

The new test was also run against the unmodified tree, where it fails with
WorkSessionStatus.COMPLETED, so it is a real regression test.

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

None. BTW stays off by default; no route, schema, configuration, or dependency
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.)
  • 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 #153 so an aborted work run is recorded as
cancelled, keeping BTW off by default and preserving authorization and request
identity.

Paths touched: astrbot/core/agent/btw/work_loop.py, tests/unit/test_btw_work_loop.py.

Checks run: the commands under Validation. The new test was first run against the
unmodified tree and failed on the reported status, then passed with the fix.

Residual risk: the loop now treats two markers as cancellation. A run that both
reports a stop and a failure still resolves to failed, unchanged from before.

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

This PR is stacked on #168 (fix/btw-dual-loop-history-152); merge that first.

run_agent reports an abort by setting agent_user_aborted and clearing
agent_stop_requested, because an aborted run must not look like a stop
request that is still pending.  WorkLoop only read the cleared flag, so a
work task the user stopped through /task stop ended as completed and
/work status reported an aborted task as finished.

Treat either marker as a cancellation.  The regression test drives the real
run_agent with only the model runner replaced, so it exercises the same
control flow that produces the markers.

Fixes #153
AI-Generated: true
Generated-At: 2026-09-11T13:29:48Z
@xero-team-bot

xero-team-bot Bot commented Sep 11, 2026

Copy link
Copy Markdown

⚠️ This PR conflicts with its base branch and needs a rebase.

git fetch origin fix/btw-dual-loop-history-152
git rebase origin/fix/btw-dual-loop-history-152
# after resolving the conflicts
git push --force-with-lease

The needs-rebase label is removed automatically once the conflicts are gone.
(Xero-Team/AstrBot · detected by xero-bot)

@YUZHEthefool
YUZHEthefool added this pull request to stack #174 September 11, 2026 15:57
@YUZHEthefool
YUZHEthefool force-pushed the fix/btw-work-cancel-on-abort-153 branch from 799394b to 03cbbc3 Compare September 11, 2026 16:00
@xero-team-bot

xero-team-bot Bot commented Sep 11, 2026

Copy link
Copy Markdown

✅ Conflicts resolved; removing the needs-rebase label.

@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