From 3b545ddfb4c42d5d5c8e0d7bb50a289e29bc0e85 Mon Sep 17 00:00:00 2001 From: mantoujun12 <202384594+mantoujun12@users.noreply.github.com> Date: Mon, 26 Oct 2026 21:08:15 +0800 Subject: [PATCH 1/4] feat(cron): let admins manage group future tasks --- astrbot/core/tools/cron_tools.py | 65 ++++++++++----- docs/en/use/proactive-agent.md | 4 +- docs/zh/use/proactive-agent.md | 4 +- tests/unit/test_cron_tools.py | 139 ++++++++++++++++++++++++++++++- 4 files changed, 185 insertions(+), 27 deletions(-) diff --git a/astrbot/core/tools/cron_tools.py b/astrbot/core/tools/cron_tools.py index 9ac05179c66..8076f44460e 100644 --- a/astrbot/core/tools/cron_tools.py +++ b/astrbot/core/tools/cron_tools.py @@ -44,6 +44,18 @@ def _job_belongs_to_current_sender( ) +def _is_group_admin(event: Any) -> bool: + """Check whether the event sender is an AstrBot admin acting in a group chat. + + Args: + event: The current message event. + + Returns: + True when the sender is an AstrBot admin and the event is a group message. + """ + return event.get_message_type() == MessageType.GROUP_MESSAGE and event.is_admin() + + def _parse_run_at(run_at: Any) -> datetime | None: if run_at in (None, ""): return None @@ -59,7 +71,8 @@ class FutureTaskTool(FunctionTool[AstrAgentContext]): "Use action='create' to schedule a recurring cron task or one-time run_at task. " "Use action='edit' to update an existing task. " "Use action='list' to inspect existing tasks. " - "Use action='delete' to remove a task by job_id." + "Use action='delete' to remove a task by job_id. " + "AstrBot admins in a group chat can list and delete tasks created by other members of that group." ) parameters: dict = Field( default_factory=lambda: { @@ -296,8 +309,11 @@ async def call( job = await cron_mgr.db.get_cron_job(str(job_id)) if not job: return f"error: cron job {job_id} not found." - if not _job_belongs_to_current_sender(job, current_umo, current_sender_id): - same_session = _extract_job_session(job) == current_umo + same_session = _extract_job_session(job) == current_umo + can_delete = _job_belongs_to_current_sender( + job, current_umo, current_sender_id + ) or (same_session and _is_group_admin(context.context.event)) + if not can_delete: if same_session and not _extract_job_sender(job): # Dashboard / legacy rows have a session but no member as # their creator, so blaming another member would be wrong. @@ -314,7 +330,8 @@ async def call( return ( f"error: cron job {job_id} was created by another member of " "this group chat, so you cannot delete it. Only the member who " - "created it can delete it; tell the user to ask that member." + "created it or an AstrBot admin can delete it; tell the user " + "to ask that member." ) return ( f"error: cron job {job_id} was not created by you, so you cannot " @@ -325,26 +342,31 @@ async def call( if action == "list": all_jobs = await cron_mgr.list_jobs() + is_group_admin = _is_group_admin(context.context.event) jobs = [ job for job in all_jobs if _job_belongs_to_current_sender(job, current_umo, current_sender_id) + or (is_group_admin and _extract_job_session(job) == current_umo) ] # Tasks in this session that were created by somebody else stay - # out of the result. Saying so stops an agent from reading "No cron - # jobs found." as "the task no longer exists". + # out of the result for regular members. Saying so stops an agent + # from reading "No cron jobs found." as "the task no longer exists". hidden_note = "" - for job in all_jobs: - if _extract_job_session(job) != current_umo: - continue - if _job_belongs_to_current_sender(job, current_umo, current_sender_id): - continue - hidden_note = ( - "\n\nNote: tasks in this chat that were not created by you " - "are not listed here, and can only be edited or deleted by " - "whoever created them." - ) - break + if not is_group_admin: + for job in all_jobs: + if _extract_job_session(job) != current_umo: + continue + if _job_belongs_to_current_sender( + job, current_umo, current_sender_id + ): + continue + hidden_note = ( + "\n\nNote: tasks in this chat that were not created by you " + "are not listed here. Only their creators can edit or delete " + "them; in group chats, AstrBot admins can also delete them." + ) + break if not jobs: return "No cron jobs found." + hidden_note tz_name = str( @@ -374,9 +396,14 @@ async def call( next_run = ( next_run.astimezone(tzinfo) if tzinfo else next_run.astimezone() ) - lines.append( - f"{j.job_id} | {j.name} | {j.job_type} | run_once={getattr(j, 'run_once', False)} | enabled={j.enabled} | next={next_run}" + line = ( + f"{j.job_id} | {j.name} | {j.job_type} | " + f"run_once={getattr(j, 'run_once', False)} | enabled={j.enabled} | " + f"next={next_run}" ) + if is_group_admin: + line += f" | creator={_extract_job_sender(j) or 'unknown'}" + lines.append(line) return "\n".join(lines) + hidden_note return "error: action must be one of create, edit, delete, or list." diff --git a/docs/en/use/proactive-agent.md b/docs/en/use/proactive-agent.md index 92c8f27cf6b..6371a884f9f 100644 --- a/docs/en/use/proactive-agent.md +++ b/docs/en/use/proactive-agent.md @@ -41,7 +41,7 @@ You can also ask it to: - “Move my break reminder to 3 PM tomorrow.” - “Delete the news task I just created.” -Chat-based listing, editing, and deletion are limited to tasks created by **the current sender in the current conversation**. Group members cannot edit or delete each other's tasks. Tasks created in the WebUI do not belong to a chat sender; administrators should manage them in the WebUI. +Chat-based listing, editing, and deletion default to tasks created by **the current sender in the current conversation**, and regular group members cannot manage each other's tasks. AstrBot admins in a group chat are the exception: they can list every task in the current group (including its creator) and delete tasks created by other members; editing remains limited to the creator. Admins can also delete same-group tasks created in the WebUI that have no recorded chat creator. ## Create and manage tasks in the WebUI @@ -119,4 +119,4 @@ Create a one-off task a few minutes ahead that sends a short test reminder. Conf | Task ran but no message arrived | Check the delivery target, platform support, connection, and send permissions. No target means no automatic delivery. | | Search or file operations fail | Test those tools' configuration, dependencies, and permissions separately. Scheduling only wakes the Agent. | | A one-off task disappeared | Scheduled execution deletes it even on failure. Inspect logs for the outcome. | -| Chat cannot find an existing task | Query as the original sender in the original conversation. Use the WebUI for tasks created by others or by the dashboard. | +| Chat cannot find an existing task | Query as the original sender in the original conversation. AstrBot admins can list all tasks in the current group and delete tasks created by other members; otherwise use the WebUI. | diff --git a/docs/zh/use/proactive-agent.md b/docs/zh/use/proactive-agent.md index 0cd321ff2a2..ca17616fc5f 100644 --- a/docs/zh/use/proactive-agent.md +++ b/docs/zh/use/proactive-agent.md @@ -41,7 +41,7 @@ - “把我的休息提醒改到明天下午三点。” - “删除我刚才设置的新闻任务。” -聊天中的任务查询、修改和删除限定为**当前会话、当前发送者创建的任务**。群成员不能修改或删除别人创建的任务;从 WebUI 创建的任务也不能直接在聊天中当作自己的任务管理,管理员应使用 WebUI。 +聊天中的任务查询、修改和删除默认限定为**当前会话、当前发送者创建的任务**,普通群成员不能操作别人创建的任务。AstrBot 管理员在群聊中例外:可以列出本群会话的全部任务(含创建者),并删除其他成员创建的任务;编辑仍仅限创建者。从 WebUI 创建且未记录创建者的任务,管理员也可在对应群聊中删除。 ## 通过 WebUI 创建和管理 @@ -119,4 +119,4 @@ AstrBot 内置 `send_message_to_user` 工具,支持文本、图片、语音、 | 执行了但没有消息 | 检查投递地、支持的平台、平台连接和发送权限;未设置投递地不会自动回传结果。 | | 搜索或文件操作失败 | 单独验证相应工具的配置、依赖和权限;主动型能力只负责定时唤醒。 | | 一次性任务不见了 | 定时触发后无论成功失败都会删除,到日志中检查执行结果。 | -| 聊天里找不到某个已有任务 | 确认在原会话由同一个发送者查询;其他成员或 WebUI 创建的任务需要到 WebUI 管理。 | +| 聊天里找不到某个已有任务 | 确认在原会话由同一个发送者查询。AstrBot 管理员可在群聊中查看本群任务并删除他人任务;其他情况请到 WebUI 管理。 | diff --git a/tests/unit/test_cron_tools.py b/tests/unit/test_cron_tools.py index 827124857c3..08b027c69ce 100644 --- a/tests/unit/test_cron_tools.py +++ b/tests/unit/test_cron_tools.py @@ -17,6 +17,7 @@ def _context( sender_id: str = "user-1", tz_name: str | None = "Asia/Shanghai", message_type: MessageType = MessageType.GROUP_MESSAGE, + is_admin: bool = False, ): return SimpleNamespace( context=SimpleNamespace( @@ -28,6 +29,7 @@ def _context( unified_msg_origin=umo, get_sender_id=lambda: sender_id, get_message_type=lambda: message_type, + is_admin=lambda: is_admin, ), ) ) @@ -212,12 +214,140 @@ async def test_future_task_delete_rejects_same_umo_different_sender(): assert result == ( "error: cron job job-1 was created by another member of this group chat, " - "so you cannot delete it. Only the member who created it can delete it; " - "tell the user to ask that member." + "so you cannot delete it. Only the member who created it or an AstrBot " + "admin can delete it; tell the user to ask that member." ) cron_mgr.delete_job.assert_not_awaited() +@pytest.mark.asyncio +async def test_future_task_admin_can_delete_other_members_group_job(): + """AstrBot admins should be able to delete another member's task in the group.""" + tool = FutureTaskTool() + existing_job = _job("job-1", sender_id="user-2") + cron_mgr = SimpleNamespace( + db=SimpleNamespace(get_cron_job=AsyncMock(return_value=existing_job)), + delete_job=AsyncMock(), + ) + + result = await tool.call( + _context(cron_mgr, sender_id="admin-user", is_admin=True), + action="delete", + job_id="job-1", + ) + + assert result == "Deleted cron job job-1." + cron_mgr.delete_job.assert_awaited_once_with("job-1") + + +@pytest.mark.asyncio +async def test_future_task_admin_cannot_delete_job_from_another_session(): + """Admin overrides stay scoped to the current group session.""" + tool = FutureTaskTool() + existing_job = _job("job-1", umo="test:group:other", sender_id="user-2") + cron_mgr = SimpleNamespace( + db=SimpleNamespace(get_cron_job=AsyncMock(return_value=existing_job)), + delete_job=AsyncMock(), + ) + + result = await tool.call( + _context(cron_mgr, sender_id="admin-user", is_admin=True), + action="delete", + job_id="job-1", + ) + + assert result == ( + "error: cron job job-1 was not created by you, so you cannot delete it. " + "Only whoever created it can delete it." + ) + cron_mgr.delete_job.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_future_task_admin_cannot_edit_other_members_group_job(): + """Admin overrides only cover listing and deleting, not editing.""" + tool = FutureTaskTool() + existing_job = _job("job-1", sender_id="user-2") + cron_mgr = SimpleNamespace( + db=SimpleNamespace(get_cron_job=AsyncMock(return_value=existing_job)), + update_job=AsyncMock(), + ) + + result = await tool.call( + _context(cron_mgr, sender_id="admin-user", is_admin=True), + action="edit", + job_id="job-1", + note="admin note", + ) + + assert result == ( + "error: cron job job-1 was created by another member of this group chat, " + "so you cannot edit it. Only the member who created it can edit it; tell " + "the user to ask that member." + ) + cron_mgr.update_job.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_future_task_group_admin_override_requires_a_group_message(): + """Admin overrides must not apply outside group chats.""" + tool = FutureTaskTool() + existing_job = _job( + "job-1", + umo="test:private:shared", + sender_id="user-2", + ) + cron_mgr = SimpleNamespace( + db=SimpleNamespace(get_cron_job=AsyncMock(return_value=existing_job)), + delete_job=AsyncMock(), + ) + + result = await tool.call( + _context( + cron_mgr, + umo="test:private:shared", + sender_id="admin-user", + message_type=MessageType.FRIEND_MESSAGE, + is_admin=True, + ), + action="delete", + job_id="job-1", + ) + + assert result == ( + "error: cron job job-1 was not created by you, so you cannot delete it. " + "Only whoever created it can delete it." + ) + cron_mgr.delete_job.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_future_task_list_admin_sees_group_tasks_with_creator(): + """Admins should list every task in the current group and see its creator.""" + tool = FutureTaskTool() + admin_job = _job("admin-job", sender_id="admin-user") + member_job = _job("member-job", sender_id="user-2") + other_session_job = _job( + "other-session-job", + umo="test:group:other", + sender_id="user-2", + ) + cron_mgr = SimpleNamespace( + list_jobs=AsyncMock(return_value=[admin_job, member_job, other_session_job]) + ) + + result = await tool.call( + _context(cron_mgr, sender_id="admin-user", is_admin=True), + action="list", + ) + + assert "admin-job" in result + assert "member-job" in result + assert "creator=user-2" in result + assert "other-session-job" not in result + assert "Note:" not in result + + @pytest.mark.asyncio async def test_future_task_list_filters_by_umo_and_sender(): """List mode should show only tasks owned by the current sender.""" @@ -438,8 +568,9 @@ async def test_future_task_list_notes_hidden_same_session_group_tasks(): assert "own-job" in result assert "other-member-job" not in result assert ( - "Note: tasks in this chat that were not created by you are not listed here, " - "and can only be edited or deleted by whoever created them." + "Note: tasks in this chat that were not created by you are not listed here. " + "Only their creators can edit or delete them; in group chats, AstrBot admins " + "can also delete them." ) in result From a3b79d1b32bafbf68eab6e93b714a913b987084d Mon Sep 17 00:00:00 2001 From: mantoujun12 <202384594+mantoujun12@users.noreply.github.com> Date: Mon, 26 Oct 2026 21:37:07 +0800 Subject: [PATCH 2/4] refactor(cron): inline group admin permission check --- astrbot/core/tools/cron_tools.py | 34 +++++++++++++------------------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/astrbot/core/tools/cron_tools.py b/astrbot/core/tools/cron_tools.py index 8076f44460e..9069abd6f87 100644 --- a/astrbot/core/tools/cron_tools.py +++ b/astrbot/core/tools/cron_tools.py @@ -44,18 +44,6 @@ def _job_belongs_to_current_sender( ) -def _is_group_admin(event: Any) -> bool: - """Check whether the event sender is an AstrBot admin acting in a group chat. - - Args: - event: The current message event. - - Returns: - True when the sender is an AstrBot admin and the event is a group message. - """ - return event.get_message_type() == MessageType.GROUP_MESSAGE and event.is_admin() - - def _parse_run_at(run_at: Any) -> datetime | None: if run_at in (None, ""): return None @@ -204,8 +192,9 @@ async def call( ) return f"Scheduled future task {job.job_id} ({job.name}) {suffix}." - current_umo = context.context.event.unified_msg_origin - current_sender_id = str(context.context.event.get_sender_id()) + event = context.context.event + current_umo = event.unified_msg_origin + current_sender_id = str(event.get_sender_id()) if action == "edit": job_id = kwargs.get("job_id") if not job_id: @@ -231,8 +220,7 @@ async def call( ) if ( same_session - and context.context.event.get_message_type() - == MessageType.GROUP_MESSAGE + and event.get_message_type() == MessageType.GROUP_MESSAGE ): return ( f"error: cron job {job_id} was created by another member of " @@ -312,7 +300,11 @@ async def call( same_session = _extract_job_session(job) == current_umo can_delete = _job_belongs_to_current_sender( job, current_umo, current_sender_id - ) or (same_session and _is_group_admin(context.context.event)) + ) or ( + same_session + and event.get_message_type() == MessageType.GROUP_MESSAGE + and event.is_admin() + ) if not can_delete: if same_session and not _extract_job_sender(job): # Dashboard / legacy rows have a session but no member as @@ -324,8 +316,7 @@ async def call( ) if ( same_session - and context.context.event.get_message_type() - == MessageType.GROUP_MESSAGE + and event.get_message_type() == MessageType.GROUP_MESSAGE ): return ( f"error: cron job {job_id} was created by another member of " @@ -342,7 +333,10 @@ async def call( if action == "list": all_jobs = await cron_mgr.list_jobs() - is_group_admin = _is_group_admin(context.context.event) + is_group_admin = ( + event.get_message_type() == MessageType.GROUP_MESSAGE + and event.is_admin() + ) jobs = [ job for job in all_jobs From 381c046febb7ca1744628be5415c6d022382eed0 Mon Sep 17 00:00:00 2001 From: mantoujun12 <202384594+mantoujun12@users.noreply.github.com> Date: Mon, 26 Oct 2026 21:37:09 +0800 Subject: [PATCH 3/4] fix(cron): block scheduled runs after deletion begins --- astrbot/core/cron/manager.py | 55 +++++++++++----- tests/unit/test_cron_manager.py | 109 ++++++++++++++++++++++++++++++++ 2 files changed, 147 insertions(+), 17 deletions(-) diff --git a/astrbot/core/cron/manager.py b/astrbot/core/cron/manager.py index 92d5322eceb..e73a0ffa44e 100644 --- a/astrbot/core/cron/manager.py +++ b/astrbot/core/cron/manager.py @@ -112,6 +112,10 @@ def __init__(self, db: BaseDatabase) -> None: # The scheduler may start early via _schedule_job; track DB sync separately. self._db_synced = False self._running_tasks: dict[str, set[asyncio.Task]] = {} + # Ref-counted deletion markers. A run must not register after deletion + # has started, and nested/concurrent deletes must not clear the marker + # before the last deletion finishes. + self._deleting_jobs: dict[str, int] = {} async def start(self, ctx: "Context") -> None: self.ctx: Context = ctx # star context @@ -227,20 +231,31 @@ async def update_job(self, job_id: str, **kwargs) -> CronJob | None: async def delete_job(self, job_id: str) -> None: """Remove a schedule and drain its active runs before deleting its row. + Runs that have already started are cancelled and drained. Runs that + start after deletion begins return without executing. + Args: job_id: Job to delete, including any currently executing run. """ - self._remove_scheduled(job_id) - self._basic_handlers.pop(job_id, None) - tasks = [ - task - for task in self._running_tasks.get(job_id, set()) - if task is not asyncio.current_task() and not task.done() - ] - for task in tasks: - task.cancel() - await asyncio.gather(*tasks, return_exceptions=True) - await self.db.delete_cron_job(job_id) + self._deleting_jobs[job_id] = self._deleting_jobs.get(job_id, 0) + 1 + try: + self._remove_scheduled(job_id) + self._basic_handlers.pop(job_id, None) + tasks = [ + task + for task in self._running_tasks.get(job_id, set()) + if task is not asyncio.current_task() and not task.done() + ] + for task in tasks: + task.cancel() + await asyncio.gather(*tasks, return_exceptions=True) + await self.db.delete_cron_job(job_id) + finally: + remaining = self._deleting_jobs.get(job_id, 0) - 1 + if remaining > 0: + self._deleting_jobs[job_id] = remaining + else: + self._deleting_jobs.pop(job_id, None) async def list_jobs(self, job_type: str | None = None) -> list[CronJob]: return await self.db.list_cron_jobs(job_type) @@ -361,12 +376,16 @@ async def _run_job( ignore_enabled: bool = False, delete_run_once: bool = True, ) -> None: - job = await self.db.get_cron_job(job_id) - if not job or (not job.enabled and not ignore_enabled): - return task = asyncio.current_task() + if task is None or job_id in self._deleting_jobs: + return + # Register before the first await so delete_job always snapshots a run + # that has passed the deletion-marker check. self._running_tasks.setdefault(job_id, set()).add(task) try: + job = await self.db.get_cron_job(job_id) + if not job or (not job.enabled and not ignore_enabled): + return start_time = datetime.now(timezone.utc) await self.db.update_cron_job( job_id, status="running", last_run_at=start_time, last_error=None @@ -400,9 +419,11 @@ async def _run_job( # One-shot runs remove themselves, excluding their own task. await self.delete_job(job_id) finally: - self._running_tasks[job_id].discard(task) - if not self._running_tasks[job_id]: - del self._running_tasks[job_id] + running = self._running_tasks.get(job_id) + if running is not None: + running.discard(task) + if not running: + del self._running_tasks[job_id] async def _run_basic_job(self, job: CronJob) -> None: handler = self._basic_handlers.get(job.job_id) diff --git a/tests/unit/test_cron_manager.py b/tests/unit/test_cron_manager.py index ebee59eeace..d18d714e307 100644 --- a/tests/unit/test_cron_manager.py +++ b/tests/unit/test_cron_manager.py @@ -1,5 +1,6 @@ """Tests for CronJobManager.""" +import asyncio import json from datetime import datetime, timedelta, timezone from types import SimpleNamespace @@ -73,6 +74,7 @@ def test_init(self, mock_db): assert manager._basic_handlers == {} assert manager._started is False assert manager._db_synced is False + assert manager._deleting_jobs == {} class TestCronJobManagerStart: @@ -315,6 +317,113 @@ async def test_delete_job(self, cron_manager, mock_db): mock_db.delete_cron_job.assert_called_once_with("test-job-id") assert "test-job-id" not in cron_manager._basic_handlers + @pytest.mark.asyncio + async def test_delete_job_cancels_run_blocked_in_row_lookup( + self, cron_manager, mock_db, sample_cron_job + ): + """A run that fetched the row must not escape deletion before registering.""" + lookup_started = asyncio.Event() + allow_lookup = asyncio.Event() + + async def delayed_get(job_id): + lookup_started.set() + await allow_lookup.wait() + return sample_cron_job + + mock_db.get_cron_job.side_effect = delayed_get + handler = MagicMock() + cron_manager._basic_handlers[sample_cron_job.job_id] = handler + + run_task = asyncio.create_task(cron_manager._run_job(sample_cron_job.job_id)) + await asyncio.wait_for(lookup_started.wait(), 2) + await asyncio.wait_for(cron_manager.delete_job(sample_cron_job.job_id), 2) + + assert run_task.cancelled() + allow_lookup.set() + handler.assert_not_called() + mock_db.delete_cron_job.assert_awaited_once_with(sample_cron_job.job_id) + assert cron_manager._running_tasks == {} + assert cron_manager._deleting_jobs == {} + + @pytest.mark.asyncio + async def test_delete_job_blocks_run_starting_after_deletion_begins( + self, cron_manager, mock_db, sample_cron_job + ): + """A run must not start once the job deletion marker is set.""" + delete_started = asyncio.Event() + allow_delete = asyncio.Event() + + async def delayed_delete(job_id): + delete_started.set() + await allow_delete.wait() + + mock_db.delete_cron_job.side_effect = delayed_delete + mock_db.get_cron_job.return_value = sample_cron_job + handler = MagicMock() + cron_manager._basic_handlers[sample_cron_job.job_id] = handler + + delete_task = asyncio.create_task( + cron_manager.delete_job(sample_cron_job.job_id) + ) + await asyncio.wait_for(delete_started.wait(), 2) + + await asyncio.wait_for(cron_manager._run_job(sample_cron_job.job_id), 2) + + allow_delete.set() + await asyncio.wait_for(delete_task, 2) + + handler.assert_not_called() + mock_db.get_cron_job.assert_not_awaited() + assert cron_manager._deleting_jobs == {} + + @pytest.mark.asyncio + async def test_concurrent_deletes_keep_marker_until_last_finishes( + self, cron_manager, mock_db, sample_cron_job + ): + """One deletion finishing must not unblock runs while another is pending.""" + first_delete_started = asyncio.Event() + second_delete_started = asyncio.Event() + allow_first_delete = asyncio.Event() + allow_second_delete = asyncio.Event() + delete_calls = 0 + + async def delayed_delete(job_id): + nonlocal delete_calls + delete_calls += 1 + if delete_calls == 1: + first_delete_started.set() + await allow_first_delete.wait() + else: + second_delete_started.set() + await allow_second_delete.wait() + + mock_db.delete_cron_job.side_effect = delayed_delete + mock_db.get_cron_job.return_value = sample_cron_job + handler = MagicMock() + cron_manager._basic_handlers[sample_cron_job.job_id] = handler + + first_delete = asyncio.create_task( + cron_manager.delete_job(sample_cron_job.job_id) + ) + second_delete = asyncio.create_task( + cron_manager.delete_job(sample_cron_job.job_id) + ) + await asyncio.wait_for(first_delete_started.wait(), 2) + await asyncio.wait_for(second_delete_started.wait(), 2) + + allow_first_delete.set() + await asyncio.wait_for(first_delete, 2) + assert cron_manager._deleting_jobs == {sample_cron_job.job_id: 1} + + await asyncio.wait_for(cron_manager._run_job(sample_cron_job.job_id), 2) + + allow_second_delete.set() + await asyncio.wait_for(second_delete, 2) + + handler.assert_not_called() + mock_db.get_cron_job.assert_not_awaited() + assert cron_manager._deleting_jobs == {} + class TestListJobs: """Tests for list_jobs method.""" From 2bd123717ed3b9c3695815a69fc3a3dc5f600bbd Mon Sep 17 00:00:00 2001 From: Soulter <905617992@qq.com> Date: Mon, 26 Oct 2026 23:36:27 +0800 Subject: [PATCH 4/4] feat(cron): let all group members list future tasks --- astrbot/core/tools/cron_tools.py | 23 ++++---- docs/en/use/proactive-agent.md | 4 +- docs/zh/use/proactive-agent.md | 4 +- tests/unit/test_cron_tools.py | 91 +++++++++++++++++++++----------- 4 files changed, 74 insertions(+), 48 deletions(-) diff --git a/astrbot/core/tools/cron_tools.py b/astrbot/core/tools/cron_tools.py index 9069abd6f87..ff6272e4d7e 100644 --- a/astrbot/core/tools/cron_tools.py +++ b/astrbot/core/tools/cron_tools.py @@ -60,7 +60,9 @@ class FutureTaskTool(FunctionTool[AstrAgentContext]): "Use action='edit' to update an existing task. " "Use action='list' to inspect existing tasks. " "Use action='delete' to remove a task by job_id. " - "AstrBot admins in a group chat can list and delete tasks created by other members of that group." + "Group members can list all tasks in the current group, including their creators. " + "Only task creators can edit tasks. Group members can delete only their own tasks, " + "while AstrBot admins can also delete other members' tasks in the current group." ) parameters: dict = Field( default_factory=lambda: { @@ -333,21 +335,17 @@ async def call( if action == "list": all_jobs = await cron_mgr.list_jobs() - is_group_admin = ( - event.get_message_type() == MessageType.GROUP_MESSAGE - and event.is_admin() - ) + is_group = event.get_message_type() == MessageType.GROUP_MESSAGE jobs = [ job for job in all_jobs if _job_belongs_to_current_sender(job, current_umo, current_sender_id) - or (is_group_admin and _extract_job_session(job) == current_umo) + or (is_group and _extract_job_session(job) == current_umo) ] - # Tasks in this session that were created by somebody else stay - # out of the result for regular members. Saying so stops an agent - # from reading "No cron jobs found." as "the task no longer exists". + # Private chats retain sender isolation. Explain hidden tasks so + # the agent does not mistake a filtered list for a missing task. hidden_note = "" - if not is_group_admin: + if not is_group: for job in all_jobs: if _extract_job_session(job) != current_umo: continue @@ -357,8 +355,7 @@ async def call( continue hidden_note = ( "\n\nNote: tasks in this chat that were not created by you " - "are not listed here. Only their creators can edit or delete " - "them; in group chats, AstrBot admins can also delete them." + "are not listed here. Only their creators can edit or delete them." ) break if not jobs: @@ -395,7 +392,7 @@ async def call( f"run_once={getattr(j, 'run_once', False)} | enabled={j.enabled} | " f"next={next_run}" ) - if is_group_admin: + if is_group: line += f" | creator={_extract_job_sender(j) or 'unknown'}" lines.append(line) return "\n".join(lines) + hidden_note diff --git a/docs/en/use/proactive-agent.md b/docs/en/use/proactive-agent.md index 6371a884f9f..6f2cebaf650 100644 --- a/docs/en/use/proactive-agent.md +++ b/docs/en/use/proactive-agent.md @@ -41,7 +41,7 @@ You can also ask it to: - “Move my break reminder to 3 PM tomorrow.” - “Delete the news task I just created.” -Chat-based listing, editing, and deletion default to tasks created by **the current sender in the current conversation**, and regular group members cannot manage each other's tasks. AstrBot admins in a group chat are the exception: they can list every task in the current group (including its creator) and delete tasks created by other members; editing remains limited to the creator. Admins can also delete same-group tasks created in the WebUI that have no recorded chat creator. +All group members can list **every task in the current group conversation**, including its creator. Regular members can only edit and delete tasks they created in that group. AstrBot admins can also delete other members' tasks in the current group, but editing remains limited to the creator. Tasks created in the WebUI without a recorded chat creator also appear in the corresponding group's list and can be deleted by AstrBot admins. These permissions stay within the current conversation and do not allow access across groups. In private chats, listing, editing, and deletion remain limited to tasks created by the current sender in the current conversation. ## Create and manage tasks in the WebUI @@ -119,4 +119,4 @@ Create a one-off task a few minutes ahead that sends a short test reminder. Conf | Task ran but no message arrived | Check the delivery target, platform support, connection, and send permissions. No target means no automatic delivery. | | Search or file operations fail | Test those tools' configuration, dependencies, and permissions separately. Scheduling only wakes the Agent. | | A one-off task disappeared | Scheduled execution deletes it even on failure. Inspect logs for the outcome. | -| Chat cannot find an existing task | Query as the original sender in the original conversation. AstrBot admins can list all tasks in the current group and delete tasks created by other members; otherwise use the WebUI. | +| Chat cannot find an existing task | Query in the task's original conversation. All group members can list every task in their current group; private chats require the original sender. Otherwise use the WebUI. | diff --git a/docs/zh/use/proactive-agent.md b/docs/zh/use/proactive-agent.md index ca17616fc5f..641c7d629d0 100644 --- a/docs/zh/use/proactive-agent.md +++ b/docs/zh/use/proactive-agent.md @@ -41,7 +41,7 @@ - “把我的休息提醒改到明天下午三点。” - “删除我刚才设置的新闻任务。” -聊天中的任务查询、修改和删除默认限定为**当前会话、当前发送者创建的任务**,普通群成员不能操作别人创建的任务。AstrBot 管理员在群聊中例外:可以列出本群会话的全部任务(含创建者),并删除其他成员创建的任务;编辑仍仅限创建者。从 WebUI 创建且未记录创建者的任务,管理员也可在对应群聊中删除。 +群聊中的所有成员都可以查询**本群会话的全部任务**,列表会显示创建者。普通成员只能修改和删除自己在本群创建的任务;AstrBot 管理员还可以删除本群其他成员创建的任务,但编辑仍仅限创建者。从 WebUI 创建且未记录创建者的任务也会显示在对应群聊的列表中,并可由 AstrBot 管理员删除。以上权限均限定在当前会话,不能跨群管理;私聊中的查询、修改和删除仍限定为当前发送者在当前会话创建的任务。 ## 通过 WebUI 创建和管理 @@ -119,4 +119,4 @@ AstrBot 内置 `send_message_to_user` 工具,支持文本、图片、语音、 | 执行了但没有消息 | 检查投递地、支持的平台、平台连接和发送权限;未设置投递地不会自动回传结果。 | | 搜索或文件操作失败 | 单独验证相应工具的配置、依赖和权限;主动型能力只负责定时唤醒。 | | 一次性任务不见了 | 定时触发后无论成功失败都会删除,到日志中检查执行结果。 | -| 聊天里找不到某个已有任务 | 确认在原会话由同一个发送者查询。AstrBot 管理员可在群聊中查看本群任务并删除他人任务;其他情况请到 WebUI 管理。 | +| 聊天里找不到某个已有任务 | 确认在任务所属的原会话查询。所有群成员均可查看本群全部任务;私聊中需由原创建者查询。其他情况请到 WebUI 管理。 | diff --git a/tests/unit/test_cron_tools.py b/tests/unit/test_cron_tools.py index 08b027c69ce..fe95703c279 100644 --- a/tests/unit/test_cron_tools.py +++ b/tests/unit/test_cron_tools.py @@ -322,8 +322,13 @@ async def test_future_task_group_admin_override_requires_a_group_message(): @pytest.mark.asyncio -async def test_future_task_list_admin_sees_group_tasks_with_creator(): - """Admins should list every task in the current group and see its creator.""" +@pytest.mark.parametrize("is_admin", [False, True]) +async def test_future_task_list_group_members_see_tasks_with_creator(is_admin): + """All group members can see this group's tasks and their creators. + + Args: + is_admin: Whether the caller is an AstrBot administrator. + """ tool = FutureTaskTool() admin_job = _job("admin-job", sender_id="admin-user") member_job = _job("member-job", sender_id="user-2") @@ -337,7 +342,7 @@ async def test_future_task_list_admin_sees_group_tasks_with_creator(): ) result = await tool.call( - _context(cron_mgr, sender_id="admin-user", is_admin=True), + _context(cron_mgr, sender_id="admin-user", is_admin=is_admin), action="list", ) @@ -349,11 +354,18 @@ async def test_future_task_list_admin_sees_group_tasks_with_creator(): @pytest.mark.asyncio -async def test_future_task_list_filters_by_umo_and_sender(): - """List mode should show only tasks owned by the current sender.""" +@pytest.mark.parametrize("is_admin", [False, True]) +async def test_future_task_list_private_chat_filters_by_umo_and_sender(is_admin): + """Private chats retain both session and sender isolation. + + Args: + is_admin: Whether the caller is an AstrBot administrator. + """ tool = FutureTaskTool() - own_job = _job("own-job", sender_id="user-1") - same_umo_other_sender = _job("other-sender-job", sender_id="user-2") + own_job = _job("own-job", umo="test:private:shared", sender_id="user-1") + same_umo_other_sender = _job( + "other-sender-job", umo="test:private:shared", sender_id="user-2" + ) different_umo_same_sender = _job( "other-umo-job", umo="test:group:other", @@ -366,13 +378,21 @@ async def test_future_task_list_filters_by_umo_and_sender(): ) result = await tool.call( - _context(cron_mgr, sender_id="user-1"), + _context( + cron_mgr, + umo="test:private:shared", + sender_id="user-1", + message_type=MessageType.FRIEND_MESSAGE, + is_admin=is_admin, + ), action="list", ) assert "own-job" in result assert "other-sender-job" not in result assert "other-umo-job" not in result + assert "Note: tasks in this chat that were not created by you" in result + assert "creator=" not in result @pytest.mark.asyncio @@ -554,8 +574,8 @@ async def test_future_task_edit_reports_foreign_job_from_another_session(): @pytest.mark.asyncio -async def test_future_task_list_notes_hidden_same_session_group_tasks(): - """List mode must say that other members' tasks exist but are filtered out.""" +async def test_future_task_list_includes_other_members_group_tasks(): + """Regular members see everyone's tasks in the current group.""" tool = FutureTaskTool() own_job = _job("own-job", sender_id="user-1") other_member_job = _job("other-member-job", sender_id="user-2") @@ -566,25 +586,25 @@ async def test_future_task_list_notes_hidden_same_session_group_tasks(): result = await tool.call(_context(cron_mgr, sender_id="user-1"), action="list") assert "own-job" in result - assert "other-member-job" not in result - assert ( - "Note: tasks in this chat that were not created by you are not listed here. " - "Only their creators can edit or delete them; in group chats, AstrBot admins " - "can also delete them." - ) in result + assert "other-member-job" in result + assert "creator=user-1" in result + assert "creator=user-2" in result + assert "Note:" not in result @pytest.mark.asyncio -async def test_future_task_list_explains_hidden_tasks_when_none_are_owned(): - """The "no jobs" answer must not read as "your task disappeared".""" +async def test_future_task_list_shows_group_tasks_when_none_are_owned(): + """A member without tasks can still list other members' group tasks.""" tool = FutureTaskTool() other_member_job = _job("other-member-job", sender_id="user-2") cron_mgr = SimpleNamespace(list_jobs=AsyncMock(return_value=[other_member_job])) result = await tool.call(_context(cron_mgr, sender_id="user-1"), action="list") - assert result.startswith("No cron jobs found.") - assert "Note: tasks in this chat that were not created by you" in result + assert "other-member-job" in result + assert "creator=user-2" in result + assert "No cron jobs found." not in result + assert "Note:" not in result @pytest.mark.asyncio @@ -599,10 +619,17 @@ async def test_future_task_list_has_no_note_when_nothing_is_hidden(): @pytest.mark.asyncio -async def test_future_task_list_ignores_other_sessions_for_the_hidden_note(): - """Jobs from other sessions must not leak into the count.""" +@pytest.mark.parametrize( + "payload", [{}, {"session": "test:group:other"}, {"session": "test:private:shared"}] +) +async def test_future_task_list_excludes_jobs_outside_current_group(payload): + """Only jobs explicitly assigned to the current group are visible. + + Args: + payload: A job payload without the current group's session. + """ tool = FutureTaskTool() - other_session_job = _job("other-session-job", umo="test:group:other") + other_session_job = _raw_job("other-session-job", payload=payload) cron_mgr = SimpleNamespace(list_jobs=AsyncMock(return_value=[other_session_job])) result = await tool.call(_context(cron_mgr, sender_id="user-1"), action="list") @@ -611,8 +638,8 @@ async def test_future_task_list_ignores_other_sessions_for_the_hidden_note(): @pytest.mark.asyncio -async def test_future_task_list_notes_hidden_basic_job_in_the_session(): - """A basic job in this session is still not a task of the caller.""" +async def test_future_task_list_includes_basic_job_in_the_group(): + """A basic job assigned to the current group is visible to members.""" tool = FutureTaskTool() basic_job = _raw_job( "basic-job", @@ -623,13 +650,14 @@ async def test_future_task_list_notes_hidden_basic_job_in_the_session(): result = await tool.call(_context(cron_mgr, sender_id="user-1"), action="list") - assert result.startswith("No cron jobs found.") - assert "not created by you are not listed here" in result + assert "basic-job" in result + assert "creator=unknown" in result + assert "Note:" not in result @pytest.mark.asyncio -async def test_future_task_list_notes_hidden_jobs_without_a_creator(): - """Dashboard/legacy rows have a session but no member as their creator.""" +async def test_future_task_list_includes_group_jobs_without_a_creator(): + """Dashboard or legacy jobs assigned to the group remain visible.""" tool = FutureTaskTool() orphan_job = _raw_job( "orphan-job", @@ -639,8 +667,9 @@ async def test_future_task_list_notes_hidden_jobs_without_a_creator(): result = await tool.call(_context(cron_mgr, sender_id="user-1"), action="list") - assert result.startswith("No cron jobs found.") - assert "not created by you are not listed here" in result + assert "orphan-job" in result + assert "creator=unknown" in result + assert "Note:" not in result @pytest.mark.asyncio