fix(channels): explain in-flight goal write rejections (#7741)

* fix(channels): explain in-flight goal write rejections

* test(channels): pin Gateway busy-goal feedback contract

---------

Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
Daoyuan Li 2026-11-01 17:16:19 -08:00 committed by GitHub
parent 08f343f5d7
commit 80902c52b7
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 124 additions and 4 deletions

View File

@ -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 <completion condition>` 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 <completion condition>` 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 <completion condition>` 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.

View File

@ -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.

View File

@ -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": "当前任务仍在运行,请等待结束后再修改目标。",
},
}

View File

@ -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")

View File

@ -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.

View File

@ -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.")],

View File

@ -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"