refactor: drop persist_session plumbing + fix broken btw mid-turn bypass (#16075)
Follow-up to PR #16053 (/btw as /background alias). Cleans up the plumbing added exclusively for the old ephemeral /btw handler and repairs a broken btw bypass that landed between my refactor and this follow-up. run_agent.py: - Remove persist_session kwarg, instance attr, and _persist_session short-circuit. Only /btw ever passed persist_session=False; with /btw gone the default (always persist) is the only behavior anyone ever wanted. gateway/run.py: - Remove the unreachable 'if _cmd_def_inner.name == "btw"' block (PR #16059). Canonical name for a /btw message is 'background' after alias resolution — the comparison could never be true, and it called _handle_btw_command which no longer exists. The /background branch above it already dispatches /btw correctly. tests/gateway/test_running_agent_session_toggles.py: - Fix test_btw_dispatches_mid_run to mock _handle_background_command (the real dispatch target for /btw) instead of the deleted _handle_btw_command.
This commit is contained in:
@@ -169,23 +169,22 @@ async def test_reasoning_rejected_mid_run():
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_btw_dispatches_mid_run():
|
||||
"""/btw mid-run must dispatch to its handler, not hit the catch-all.
|
||||
"""/btw mid-run must dispatch to /background's handler, not hit the catch-all.
|
||||
|
||||
/btw spawns a parallel ephemeral side-question task that does NOT
|
||||
interrupt the active conversation (see _handle_btw_command). It's the
|
||||
whole point of the command — asking a side question while the main
|
||||
turn is still working. Before the mid-turn bypass was added, /btw
|
||||
fell through to the "Agent is running — wait or /stop first" catch-all,
|
||||
making it useless in exactly the scenario it was designed for.
|
||||
/btw is an alias of /background (see hermes_cli/commands.py). Typing
|
||||
/btw mid-turn must spawn a parallel background task — that's the whole
|
||||
point of the command. Before the mid-turn bypass was added for
|
||||
/background, /btw fell through to the "Agent is running — wait or
|
||||
/stop first" catch-all, making it useless in exactly the scenario it
|
||||
was designed for. The alias and the bypass together make it work.
|
||||
"""
|
||||
runner = _make_runner()
|
||||
runner._handle_btw_command = AsyncMock(
|
||||
return_value='💬 /btw: "what module owns titles?"\nReply will appear here shortly.'
|
||||
runner._handle_background_command = AsyncMock(
|
||||
return_value='🚀 Background task started: "what module owns titles?"'
|
||||
)
|
||||
|
||||
result = await runner._handle_message(_make_event("/btw what module owns titles?"))
|
||||
|
||||
runner._handle_btw_command.assert_awaited_once()
|
||||
runner._handle_background_command.assert_awaited_once()
|
||||
assert result is not None
|
||||
assert "💬 /btw" in result
|
||||
assert "can't run mid-turn" not in result
|
||||
|
||||
Reference in New Issue
Block a user