Skip to content

fix(btw): compensate a work submission that is never handed over - #171

Open
YUZHEthefool wants to merge 1 commit into
fix/btw-work-failure-detection-154from
fix/btw-submit-handoff-compensation-155
Open

fix(btw): compensate a work submission that is never handed over#171
YUZHEthefool wants to merge 1 commit into
fix/btw-work-failure-detection-154from
fix/btw-submit-handoff-compensation-155

Conversation

@YUZHEthefool

@YUZHEthefool YUZHEthefool commented Sep 11, 2026

Copy link
Copy Markdown
Member

Summary

A work submission interrupted between its acknowledgement and the background
hand-off left a queued task no run owned. Expected the session to end
cancelled/failed and be cleaned up once; actual /work status kept reporting
a pending task, with zero background tasks registered and the event already
cleaned up. Pending sessions are exempt from retention, so the record survived
until a reload or restart.

Related issue

Fixes #155

Root cause

submit() creates the queued session, yields the acknowledgement, and registers
the background run only once the generator resumes. When a later stage stops the
event, the scheduler breaks out of the generator and never resumes it — an
async for that breaks does not close its generator — so no code path cancelled
the session. close() only walks registered _tasks, and the retention sweep
skips PENDING, so nothing reaped it either.

Reproduction

Build a real PipelineScheduler and WorkLoop, add a stage after the
acknowledgement that calls event.stop_event(), then run scheduler.execute and
read the WorkSession. The production equivalents are a plugin that stops the
event while decorating the acknowledgement, or a cancelled request.

Implementation notes

Reclaim the session from both ends, with the transition itself deciding who did
it:

  • submit() cancels the session in a finally unless the background run
    accepted it. This covers a generator closed by aclose()/GeneratorExit.
  • PipelineScheduler.execute's existing finally is the one place that runs even
    when the generator is never resumed, so it asks runtime_registry to cancel a
    work session whose background run never took over. This reuses the same hook
    point the scheduler already has for BTW (btw_detached_work,
    finalize_detached_event), so the coupling is not new.
  • WorkSessionManager.cancel_if_pending only moves PENDING to CANCELLED, so
    the two paths cannot cancel twice or clean up twice. It cannot touch a
    legitimate background run that is merely queued behind the semaphore, because
    the hand-off marks the event before the task starts.
  • config_id_of moves to runtime_registry (the module the pipeline already uses
    to reach work-loop state); submission imports it from there.

Validation

.venv/Scripts/python.exe -m pytest tests/unit -q
  5220 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 run against the unmodified tree first, where it fails with
WorkSessionStatus.PENDING and zero background tasks — the exact reported
symptom.

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

  • PipelineScheduler.execute's finally now makes one extra call per event
    (a registry lookup plus a mark check). It is a no-op for every event that does
    not carry a BTW work session.
  • config_id_of is still importable from astrbot.core.agent.btw.submission.
  • No route, schema, configuration, or dependency change. BTW stays 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. (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 #155 by compensating between the acknowledgement
and the background hand-off, keeping BTW off by default and preserving
authorization and request identity.

Paths touched: astrbot/core/agent/btw/work_loop.py,
astrbot/core/agent/btw/work_sessions.py,
astrbot/core/agent/btw/runtime_registry.py,
astrbot/core/agent/btw/submission.py, astrbot/core/pipeline/scheduler.py, and
tests/unit/test_btw_delivery.py, tests/unit/test_work_tools.py.

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

Residual risk: the compensation depends on the scheduler's finally running. An
isolated WorkLoop used with submit() and no scheduler has neither the
generator resumption nor that backstop, so a submission abandoned there could
still linger; the finally in submit() covers the close paths that do occur,
and no such caller exists in the tree.

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

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

submit() creates the queued session, yields the acknowledgement, and only
registers the background run once the generator resumes.  A later stage that
stops the event drops the generator instead, so nothing cancelled the session:
/work status kept reporting a queued task that no run owned, and pending
sessions are exempt from retention, so it survived until a reload.

Reclaim the session from both ends.  The generator cancels it in a finally
unless the background run accepted it, and the scheduler -- the only place
that runs even when the generator is never resumed -- cancels it as the event
finishes.  cancel_if_pending makes the transition itself decide which caller
performs it, so the two paths cannot cancel or clean up twice.

config_id_of moves to runtime_registry, which is already how the pipeline
reaches work-loop state.

Fixes #155
AI-Generated: true
Generated-At: 2026-09-11T14:02:36Z
@YUZHEthefool
YUZHEthefool added this pull request to stack #174 September 11, 2026 15:57
@YUZHEthefool
YUZHEthefool force-pushed the fix/btw-submit-handoff-compensation-155 branch from e890c8a to c729ccd Compare September 11, 2026 16:00
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