Skip to content

fix(btw): separate loop histories and submit work with a tool - #168

Merged
YUZHEthefool merged 3 commits into
masterfrom
fix/btw-dual-loop-history-152
Sep 14, 2026
Merged

fix(btw): separate loop histories and submit work with a tool#168
YUZHEthefool merged 3 commits into
masterfrom
fix/btw-dual-loop-history-152

Conversation

@YUZHEthefool

Copy link
Copy Markdown
Member

Summary

BTW's conversation loop and work loop shared one conversation, so a detached
work run and a chat run each committed a history snapshot that was read before
the other wrote. Whichever finished second won: the later commit replaced the
earlier turn, and when the later commit was the older in-process sequence it was
rejected as stale instead. A finished turn went missing either way.

Expected: both replies become complete history turns, and ordinary dialogue does
not wait for the background task.

Actual (issue #152): sequential commits return [true, true] but the later one
overwrites the earlier, and the reverse order returns [true, false], discarding
the earlier run; only the second sequence's turns survive.

This change keeps two independent histories instead of merging them. A session
now owns one conversation per loop, a work run starts from no history of its own,
and the task reaches the work loop as an explicit tool call from the conversation
loop. The conversation loop can still read what the work loop produced.

Related issue

Fixes #152

Root cause

Every agent run resolves the session's current conversation, and both loops wrote
through the same AssistantHistoryCommitter, whose per-conversation sequence
guard exists to stop an older snapshot from replacing newer data. But the two
loops hold different session locks (<umo> for the dialogue loop, <umo>:work
for the work loop), so they genuinely overlap, and each one builds its snapshot
from the history it read before the other committed. The guard then had only two
outcomes, both lossy: overwrite with the newer sequence, or drop the older one.

Merging the two snapshots would keep both turns but leaves the two loops writing
one history, which is what the dual-loop split exists to avoid. Separating the
histories removes the conflict by construction.

Reproduction

  1. Enable BTW and the work loop, keep group_sender_concurrency at its default
    false, and use a local Agent.
  2. In one existing session, submit a /work task that finishes slowly, then send
    an ordinary chat message so both runs read the same initial history.
  3. Let the work task finish first and the dialogue second (and repeat with the
    order reversed), then read /conversation history.

Before this change only the second sequence's turn remains in both orders. The
regression tests drive the same control flow: two runs over one in-memory
conversation store, released in both completion orders, asserting that both
complete turns are stored.

Implementation notes

  • conversation_mgr: each loop selects its own current conversation
    (sel_conv_id, sel_conv_id:work). The work conversation is created on the
    work loop's first run and belongs to the same session, so both appear in the
    session's conversation list. The parameter defaults to the dialogue scope, so
    every existing caller keeps today's behavior.
  • _get_session_conv(scope) resolves per loop; _prepare_loop_contexts gives a
    work run an empty history and gives the dialogue run the work loop's recent
    complete turns as provider-only context. Those turns carry _no_save and are
    dropped by the existing _save_to_history filter, so neither stored history
    receives the other's content. Turns are reduced to their text roles: a tool
    call without its paired result would be rejected by the provider.
  • The work loop's task is explicit. submit_work_task submits a model-authored
    prompt, returns immediately, and the run is delivered through the existing
    detached-work result path. The handed-over run is a new event that mirrors the
    requesting event's subject, instance role, and auth resource but carries a
    fresh request id and no consumed WebChat step-up proof.
  • With BTW disabled nothing changes: no work conversation is created and a run
    reads its conversation history exactly as before.

Validation

.venv/Scripts/python.exe -m pytest tests/unit -q
  5212 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
prettier --check / markdownlint-cli2 on the two updated docs      # clean

make check, make quality (bandit, radon), and the full
pytest --test-profile blocking profile were not run locally on this Windows
checkout; this PR relies on CI for those.

Compatibility and risk

  • New tool submit_work_task, offered only to the conversation loop of a profile
    that enables BTW and the work loop, and only to callers allowed
    agent.manage (the same action send_message_to_user requires). The work loop
    cannot submit to itself.
  • Behavior change: a work run no longer reads the chat history, by design. Its
    input is the explicit task, either the /work <text> argument or the prompt the
    conversation loop supplies through the tool. A caller that expects the work loop
    to see earlier chat must put that context in the prompt.
  • New public API btw_submit_work_task; get_curr_conversation_id and
    new_conversation gained a defaulted scope parameter.
  • No route, schema, OpenAPI, configuration, or migration changes. BTW remains off
    by default.

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 secrets committed. Runtime Python deps update pyproject.toml, requirements.txt, and uv.lock together.
  • 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.
  • 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 #152 with the dual-loop history ownership the
report's acceptance criteria ask for, keeping BTW off by default and preserving
authorization and request identity.

Paths touched: astrbot/core/conversation_mgr.py,
astrbot/core/astr_main_agent.py,
astrbot/core/agent/btw/{submission,work_loop,runtime_registry}.py,
astrbot/core/tools/{work_tools,registry}.py, astrbot/core/tool_catalog.py,
astrbot/core/pipeline/process_stage/stage.py, astrbot/api/__init__.py,
docs/{zh,en}/dev/astrbot-config.md, and unit tests
(test_work_tools.py is new; test_btw_work_loop.py,
test_btw_capability_routes.py, test_astr_main_agent.py,
test_conversation_manager.py, and test_agent_internal_history.py are
extended).

Checks run: the commands listed under Validation above. The new dual-loop test
was also checked against the unmodified tree, where it fails for both completion
orders, so it is a real regression test rather than a description of current
behavior.

Residual risk: result delivery for a tool-submitted run reuses the detached-work
path with a synthetic event, which unit tests cover end to end through a stub
executor, but no live platform adapter was exercised here. The /work command
keeps its explicit-argument semantics and therefore no longer implies the chat
history; that is the intended design but is worth a maintainer's eye.

Tools used: Claude Code (Opus 5) with the repository's AGENTS.md, AI_POLICY.md,
and conventional-commit references. Existing code was read directly, and the
reported symptoms were reproduced against the real AssistantHistoryCommitter
before the fix.

@YUZHEthefool

Copy link
Copy Markdown
Member Author

@xero-team-bot r? @BegoniaHe

@xero-team-bot

xero-team-bot Bot commented Sep 11, 2026

Copy link
Copy Markdown

✅ Requested a review from @BegoniaHe.
✅ Assigned @BegoniaHe 🙏

YUZHEthefool and others added 3 commits September 11, 2026 17:16
The conversation loop and the work loop shared the session's conversation,
so a detached work run and a chat run each committed a history snapshot
read before the other wrote. Whichever finished second replaced the first,
and a completion that carried an older in-process sequence was dropped as
stale, so a finished turn went missing from the stored history (#152).

Keep two histories instead of merging them. A session now owns one
conversation per loop: the dialogue loop keeps the session's current
conversation, and the work loop owns a second one, created on its first
run and listed next to the chat under the same session. Work runs start
from no history of their own, and the dialogue loop reads the work loop's
recent complete turns as provider-only context, so the chat can refer to
work results without either side entering the other's stored history.
Handed-over turns are reduced to their text roles because tool calls would
otherwise reach a provider without their paired results.

Disabling BTW changes nothing: no work conversation is created, and a run
reads its conversation history exactly as before.

Refs #152
AI-Generated: true
Generated-At: 2026-09-11T11:52:57Z
The work loop keeps a history-free conversation, so it needs the task itself
rather than the chat it came from. Hand the task over the way an agent hands
work to a tool: the conversation loop's model calls `submit_work_task` with a
complete task prompt, the tool submits it and returns immediately, and the
work loop runs the task detached in its own conversation and delivers the
result through the normal result-decoration and send stages.

The tool is offered only to the conversation loop of a profile with BTW and
the work loop enabled, so the work loop cannot submit to itself. The handed
over run is a new event: it mirrors the requesting event's subject, instance
role, and auth resource, carries a fresh request id and no consumed WebChat
step-up proof, and stays registered for stop and reset until its finalizer
releases it. `btw_submit_work_task` exposes the same hand-off to plugins.

Fixes #152
AI-Generated: true
Generated-At: 2026-09-11T11:55:50Z
Break agent import cycles with deferred runtime imports and make asynchronous test awaits observable to CodeQL.

AI-Generated: true

Generated-At: 2026-09-11T14:29:55Z
@BegoniaHe
BegoniaHe force-pushed the fix/btw-dual-loop-history-152 branch from 0acb0d4 to 621585f Compare September 11, 2026 15:18
from astrbot.core.utils.task_utils import create_tracked_task

if TYPE_CHECKING:
from astrbot.core.execution_context import CoreExecutionContext

if TYPE_CHECKING:
from astrbot.core.execution_context import CoreExecutionContext
from astrbot.core.star.star import PluginRegistry
task.cancel()
with pytest.raises(asyncio.CancelledError):
await task
_cancelled_result = await task
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 工作循环与对话并发时会丢失已完成回合的历史

3 participants