Deleting a desktop session (or hitting /stop) did not stop its
background memory/skill review fork. The review is a SEPARATE AIAgent
instance, tracked only on the parent's _background_review_agent /
_active_children (agent/background_review.py) -- never on
session["running"] or session["_run_thread"]. That made it invisible
to every teardown/interrupt path:
- _teardown_popped_session's run-thread join only waits on the
FOREGROUND turn's _run_thread.
- session.interrupt's _interrupt_session_turn only calls
request_hard_interrupt when session["running"] is True -- the
common shape is the foreground turn already finished and only the
post-turn review is still running, so this branch never fires.
In the reported incident the review degraded into a failed-tool-call
retry loop (malformed arguments from a fallback local model) and kept
calling the model for 70+ minutes after the session was deleted, only
stopping on a full app restart.
Fix: call the existing
agent.background_review.cancel_background_review_for_live_turn
helper -- previously invoked only when a NEW live turn preempts a
running review -- from both:
- _finalize_session (tui_gateway/session_lifecycle.py), the single
_finalized-guarded chokepoint every teardown path (explicit close,
idle-timeout reap, ws-orphan reap, shutdown) funnels through, and
- _interrupt_session_turn, so an explicit /stop reaches a live review
even when the foreground turn has already ended.
cancel_background_review_for_live_turn gained optional
message/tool_reason kwargs (additive, backward compatible) so the new
callers report the real cause ("session ended (idle_timeout)",
"session interrupted") instead of the misleading default "superseded
by a new live turn" wording inherited from the live-turn-preemption
caller.
Two new regression test files cover the finalize path and the
/stop-only-review-running sibling path; both were proven to fail
(RED) with the fix reverted and pass (GREEN) with it applied.
Fixes#102895
Note on related PRs: #75616 and #102911 (duplicate of #75616) fix a
different, adjacent gap -- the Desktop renderer not calling
session.interrupt before session.close. That closes the window where
the FOREGROUND turn itself outlives deletion, but does not touch this
review-fork gap: in the reported log the foreground turn had already
finished (finish_reason=stop) before the review started, so
session["running"] was already False and session.interrupt's
running-gated branch would not have helped even with those PRs
applied. This fix is complementary, not a duplicate.
b797ed9779