Skip to content

fix(btw): stop a third-party work run when its request is stopped - #173

Merged
YUZHEthefool merged 1 commit into
fix/btw-work-delivery-status-156from
fix/btw-stop-third-party-work-157
Sep 14, 2026
Merged

YUZHEthefool merged 1 commit into
fix/btw-work-delivery-status-156from
fix/btw-stop-third-party-work-157

Conversation

@YUZHEthefool

@YUZHEthefool YUZHEthefool commented Sep 11, 2026

Copy link
Copy Markdown
Member

Summary

/task stop on a running third-party work task did not stop local execution:
the run kept consuming the runner's responses after the stop and finished as
completed. Expected the local wait and consumption to end, the runner to be
closed, and the task recorded cancelled.

Related issue

Fixes #157

Root cause

/task stop for a third-party runner goes through ActiveEventRegistry.stop_all,
which sets the event's stop flag — and the detached run never read it. The flag
was set the whole time while WorkLoop._run_detached pulled every response and
the third-party consumer drained the runner.

Reproduction

The regression tests build the real ActiveEventRegistry, the real
ThirdPartyAgentSubStage non-streaming handler, and a real WorkLoop, then stop
the registered work event while a stub runner is still producing. Before this
change all three responses were consumed and the session ended completed.

Implementation notes

Honour the stop on each path where the run can be waiting:

  • run_third_party_agent gained an optional should_stop predicate, polled as
    the stream is consumed; both handlers pass event.is_stopped. Optional and
    defaulted, so existing callers are unaffected.
  • A stopped run reports nothing instead of the fallback error an unfinished
    stream would otherwise produce — a stop is not a runner failure.
  • _run_detached stops dispatching delivered results and closes the execution,
    which releases the executor and the runner behind it.
  • A run still queued behind the semaphore never starts.
  • The terminal status treats a stopped event as a cancellation.

Scope, stated plainly: this ends the local wait and consumption. It does not
revoke a task the third-party service has already accepted, and the /task stop
documentation in both languages now says so rather than implying otherwise.

Validation

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

.venv/Scripts/python.exe -m pytest --test-profile blocking -q
  5349 passed, 6 skipped, 1 deselected

.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 source files>
  0 errors
node node_modules/prettier/bin/prettier.cjs --check docs/{zh,en}/use/command.md   # clean
node node_modules/markdownlint-cli2/markdownlint-cli2.mjs docs/{zh,en}/use/command.md   # exit 0

Both suites above ran on this branch's tip. The new tests were run against the
unmodified tree first: one consumed all three responses and delivered them, the
other started a task whose request had already been stopped.

pnpm run docs:build was not run locally (the docs dependencies are not
installed in this checkout), and neither were make check, make quality, or the
coverage-gated --test-profile all run; 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

  • run_third_party_agent gained an optional defaulted parameter; existing call
    sites and tests are unchanged.
  • Behavior change: a stopped third-party request no longer produces the fallback
    error result, and its work task is cancelled rather than completed.
  • Three test doubles gained the event's is_stopped/stop_event surface, which
    the stage now uses. 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.
  • 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 #157 by making an in-flight third-party work run
honour the stop request, keeping BTW off by default and preserving authorization
and request identity.

Paths touched: astrbot/core/agent/btw/work_loop.py,
astrbot/core/pipeline/process_stage/method/agent_sub_stages/third_party.py,
docs/{zh,en}/use/command.md, and unit tests
(test_btw_delivery.py extended; agent_sub_stage_support.py,
test_btw_work_loop.py, test_third_party_agent_sub_stage.py,
test_conversation_loop.py updated for the event surface the stage now uses).

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

Residual risk: the stop is observed between responses, so a runner blocked inside
a single long network call is not interrupted until that call returns. The
should_stop poll prevents pulling further responses, and closing the execution
releases the runner; neither claims the remote task was cancelled.

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

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

/task stop for a third-party runner records the stop on the event, but the
detached run never read it: it kept pulling responses from the runner and
finished as completed.  Nothing was cancelled, and the event's own stop flag
was set the whole time.

Honour the stop on all three paths.  The third-party consumer stops draining
the runner, the work loop stops dispatching delivered results and closes the
execution so the runner is released, and a run still queued behind the
semaphore never starts.

A stopped run is a cancellation, not a runner failure, so the third-party
handler reports nothing instead of the fallback error an unfinished stream
would produce.  This ends the local wait only: it does not revoke a task the
remote service already accepted, which the /task stop docs now say.

Fixes #157
AI-Generated: true
Generated-At: 2026-09-11T14:53:39Z
@YUZHEthefool
YUZHEthefool added this pull request to stack #174 September 11, 2026 15:57
@YUZHEthefool
YUZHEthefool force-pushed the fix/btw-stop-third-party-work-157 branch from 712207a to 5df3a47 Compare September 11, 2026 16:00
@YUZHEthefool
YUZHEthefool merged commit 393fd93 into master Sep 14, 2026
28 of 29 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