diff --git a/README.md b/README.md index df2f55f408..76d887d23b 100644 --- a/README.md +++ b/README.md @@ -2544,7 +2544,7 @@ For tool-using turns, successful operation results drive the no-progress count: Regenerating an earlier response keeps the goal currently set for the chat and its used continuation budget. It does not restore a goal that was cleared or achieved. The earlier answer's completion record is removed when its historical checkpoint is replayed. -The Web UI shows the active goal above the composer, says why auto-continue stopped, and shows a "Goal met" notice after a goal is met. Editing the last message is off while a goal is set. The same command is available from the TUI and supported IM channels. In IM, `/goal` also says whether the goal is in progress, why auto-continue stopped, or that it was met (until the next goal change), in the binding owner's web UI language (else `channel_connections.notification_locale`). In the Web UI and supported IM channels, setting `/goal ` also starts a run with the condition as the task; status and clear commands only manage goal state. Setting or clearing a goal is rejected while that thread has a run in flight, including a run owned by another Gateway worker, so the goal checkpoint cannot branch away from an active run's checkpoint lineage. +The Web UI shows the active goal above the composer, says why auto-continue stopped, and shows a "Goal met" notice after a goal is met. Editing the last message is off while a goal is set. The same command is available from the TUI and supported IM channels. In IM, `/goal` also says whether the goal is in progress, why auto-continue stopped, or that it was met (until the next goal change), in the binding owner's web UI language (else `channel_connections.notification_locale`). In the Web UI and supported IM channels, setting `/goal ` also starts a run with the condition as the task; status and clear commands only manage goal state. Setting or clearing a goal is rejected while that thread has a run in flight, including a run owned by another Gateway worker, so the goal checkpoint cannot branch away from an active run's checkpoint lineage. IM replies explain this rejection and ask you to wait for the task to finish; the goal stays unchanged and the rejected command is not retried automatically. When your role lacks `runs:create`, the Web UI rejects a new task or `/goal ` before preparing the thread or saving the goal, and keeps your draft for retry. Goal status, goal clearing, and `/compact` remain governed by their own endpoint permissions. diff --git a/backend/app/channels/AGENTS.md b/backend/app/channels/AGENTS.md index a125ba8657..c725e159d1 100644 --- a/backend/app/channels/AGENTS.md +++ b/backend/app/channels/AGENTS.md @@ -11,7 +11,7 @@ Telegram retries share bootstrap/Bot cleanup. Dead loops warn once; clear matching futures under `_shutdown_resources_lock` (closure unconfirmed). Finalize asyncgens; keep primary errors; propagate lone Bot errors. -**Auth**: SDK/raw-httpx `/goal` use process-local auth + `_csrf_headers()`; internal auth needs CSRF. +**Auth**: SDK/httpx `/goal` needs internal auth + `_csrf_headers()`. Localize known busy 409s; no echo/retry. DingTalk retains tasks and logs failures. diff --git a/backend/app/channels/goal_status.py b/backend/app/channels/goal_status.py index c6ee706432..188b30bf68 100644 --- a/backend/app/channels/goal_status.py +++ b/backend/app/channels/goal_status.py @@ -39,6 +39,7 @@ _TEXT: dict[str, dict[str, str]] = { "fetch_failed": "Failed to fetch goal information.", "clear_failed": "Failed to clear goal.", "set_failed": "Failed to set goal.", + "busy": "A task is still running. Wait for it to finish before changing the goal.", }, "zh-CN": { "goal": "目标：{objective}", @@ -57,6 +58,7 @@ _TEXT: dict[str, dict[str, str]] = { "fetch_failed": "无法获取目标信息。", "clear_failed": "无法清除目标。", "set_failed": "无法设置目标。", + "busy": "当前任务仍在运行，请等待结束后再修改目标。", }, } diff --git a/backend/app/channels/manager.py b/backend/app/channels/manager.py index 4a1081f849..f8656e0d53 100644 --- a/backend/app/channels/manager.py +++ b/backend/app/channels/manager.py @@ -4031,6 +4031,23 @@ class ChannelManager: self._remember_thread_model(thread_id, name) return f"Model '{name}' selected. It applies to this conversation from the next message." + @staticmethod + def _is_goal_in_flight_error(error: Exception) -> bool: + """Recognize only the Gateway's known goal-write conflict, not arbitrary 409s.""" + if not isinstance(error, httpx.HTTPStatusError) or error.response.status_code != 409: + return False + detail = { + "PUT": "Thread has a run in flight. Set the goal after the run finishes.", + "DELETE": "Thread has a run in flight. Clear the goal after the run finishes.", + }.get(error.request.method) + if detail is None: + return False + try: + body = error.response.json() + except ValueError: + return False + return isinstance(body, Mapping) and body.get("detail") == detail + async def _goal_request( self, method: str, @@ -4104,7 +4121,9 @@ class ChannelManager: return await self._goal_reply(msg, "cleared") try: await self._goal_request("delete", thread_id, headers=headers) - except Exception: + except Exception as exc: + if self._is_goal_in_flight_error(exc): + return await self._goal_reply(msg, "busy") logger.exception("Failed to clear goal through gateway") return await self._goal_reply(msg, "clear_failed") return await self._goal_reply(msg, "cleared") @@ -4114,7 +4133,9 @@ class ChannelManager: try: await self._goal_request("put", thread_id, headers=headers, json={"objective": command.objective}) - except Exception: + except Exception as exc: + if self._is_goal_in_flight_error(exc): + return await self._goal_reply(msg, "busy") logger.exception("Failed to set goal through gateway") return await self._goal_reply(msg, "set_failed") diff --git a/backend/tests/AGENTS.md b/backend/tests/AGENTS.md index 7e88e5dd75..a91edded4a 100644 --- a/backend/tests/AGENTS.md +++ b/backend/tests/AGENTS.md @@ -23,6 +23,10 @@ ownership and capture cleanup. Retain the real XLSX conversion coverage. Backend tests must preserve the runtime invariants they exercise without changing production execution topology. +`test_threads_router.py::test_goal_mutations_reject_run_owned_by_another_worker` +feeds real PUT/DELETE 409 errors into the IM classifier. Keep this contract check +so Gateway wording changes cannot silently remove localized busy feedback. + Provider cleanup tests use concrete configuration models and event gates. Surface owner-task failures while awaiting lifecycle events; join owners and event waiters during teardown. diff --git a/backend/tests/test_channels.py b/backend/tests/test_channels.py index 4713b19df2..d25bb77817 100644 --- a/backend/tests/test_channels.py +++ b/backend/tests/test_channels.py @@ -13871,6 +13871,88 @@ class TestHandleGoalCommand: _run(go()) + @pytest.mark.parametrize(("args", "method", "verb"), [("do X", "PUT", "Set"), ("clear", "DELETE", "Clear")]) + @pytest.mark.parametrize( + ("locale", "status", "payload", "busy"), + [ + ("en-US", 409, {"detail": "Thread has a run in flight. {verb} the goal after the run finishes."}, True), + ("zh-CN", 409, {"detail": "Thread has a run in flight. {verb} the goal after the run finishes."}, True), + ("en-US", 401, {"detail": "Thread has a run in flight. {verb} the goal after the run finishes."}, False), + ("en-US", 403, {"detail": "Thread has a run in flight. {verb} the goal after the run finishes."}, False), + ("en-US", 500, {"detail": "Thread has a run in flight. {verb} the goal after the run finishes."}, False), + ("en-US", 409, {"detail": "private server diagnostic"}, False), + ("en-US", 409, {"detail": "Thread has a run in flight. {other_verb} the goal after the run finishes."}, False), + ("en-US", 409, {"detail": ["private server diagnostic"]}, False), + ("en-US", 409, ["private server diagnostic"], False), + ("en-US", 409, "not JSON: private server diagnostic", False), + ], + ) + def test_goal_mutation_error_feedback(self, monkeypatch, args, method, verb, locale, status, payload, busy): + """Only known in-flight conflicts get actionable text, never raw Gateway errors.""" + from dataclasses import replace + + calls = [] + if isinstance(payload, dict) and isinstance(payload.get("detail"), str): + payload = {"detail": payload["detail"].format(verb=verb, other_verb="Clear" if verb == "Set" else "Set")} + + def respond(request): + calls.append(request) + return httpx.Response(status, text=payload) if isinstance(payload, str) else httpx.Response(status, json=payload) + + real_async_client = httpx.AsyncClient + monkeypatch.setattr( + "app.channels.manager.httpx.AsyncClient", + lambda **kw: real_async_client(transport=httpx.MockTransport(respond), **kw), + ) + + async def go(): + resolver = SimpleNamespace(locale_for=AsyncMock(return_value=locale)) + manager = self._make_manager(monkeypatch, thread_id="t-1", reply_locale=resolver) + chat = AsyncMock() + monkeypatch.setattr(manager, "_handle_chat", chat) + replies = [] + manager.bus.subscribe_outbound(AsyncMock(side_effect=replies.append)) + msg = replace(self._msg(f"/goal {args}"), owner_user_id="owner-1", connection_id="connection-1", thread_ts="reply-1") + + await manager._handle_command(msg) + + assert len(replies) == 1 + reply = replies[0] + expected = "Failed to clear goal." if method == "DELETE" else "Failed to set goal." + if busy: + expected = "当前任务仍在运行，请等待结束后再修改目标。" if locale == "zh-CN" else "A task is still running. Wait for it to finish before changing the goal." + assert reply.text == expected + assert (reply.channel_name, reply.chat_id, reply.thread_id, reply.thread_ts, reply.connection_id, reply.owner_user_id) == ("slack", "C1", "t-1", "reply-1", "connection-1", "owner-1") + assert len(calls) == 1 # No retry or follow-up run on rejection. + assert calls[0].method == method + chat.assert_not_awaited() + resolver.locale_for.assert_awaited_once_with("owner-1") + + _run(go()) + + @pytest.mark.parametrize("args", ["do X", "clear"]) + def test_goal_mutation_cancellation_is_not_reported_as_failure(self, monkeypatch, args): + async def cancelled(request): + raise asyncio.CancelledError + + real_async_client = httpx.AsyncClient + monkeypatch.setattr( + "app.channels.manager.httpx.AsyncClient", + lambda **kw: real_async_client(transport=httpx.MockTransport(cancelled), **kw), + ) + + async def go(): + manager = self._make_manager(monkeypatch, thread_id="t-1") + chat, reply = AsyncMock(), AsyncMock() + monkeypatch.setattr(manager, "_handle_chat", chat) + manager.bus.subscribe_outbound(reply) + with pytest.raises(asyncio.CancelledError): + await manager._handle_command(self._msg(f"/goal {args}")) + chat.assert_not_awaited() + reply.assert_not_awaited() + + _run(go()) + @pytest.mark.parametrize( ("args", "method", "expected_reply"), [("finish the work", "PUT", None), ("clear", "DELETE", "Goal cleared.")], diff --git a/backend/tests/test_threads_router.py b/backend/tests/test_threads_router.py index ba38318ba3..ac9c5ff1e7 100644 --- a/backend/tests/test_threads_router.py +++ b/backend/tests/test_threads_router.py @@ -1286,6 +1286,9 @@ def test_goal_status_and_clear_round_trip() -> None: def test_goal_mutations_reject_run_owned_by_another_worker() -> None: """PUT and DELETE goal writes share the durable thread-operation boundary.""" + import httpx + + from app.channels.manager import ChannelManager from deerflow.runtime import RunManager, RunStatus from deerflow.runtime.runs.store.memory import MemoryRunStore @@ -1318,6 +1321,14 @@ def test_goal_mutations_reject_run_owned_by_another_worker() -> None: assert put_response.status_code == 409, put_response.text assert delete_response.status_code == 409, delete_response.text + assert put_response.json()["detail"] == "Thread has a run in flight. Set the goal after the run finishes." + assert delete_response.json()["detail"] == "Thread has a run in flight. Clear the goal after the run finishes." + # The real Gateway errors must remain recognizable by the IM busy reply. + for response in (put_response, delete_response): + with pytest.raises(httpx.HTTPStatusError) as rejected: + response.raise_for_status() + assert ChannelManager._is_goal_in_flight_error(rejected.value), response.request.method + assert goal_response.status_code == 200, goal_response.text assert goal_response.json()["goal"]["objective"] == "Original goal"