Skip to content

Commit b6d1d42

Browse files
author
xiami762
committed
fix(task): address PR #135 review findings
- task_center: infer scheduled task when run_once=True so missing run_at/cron raises a proper validation error instead of silently executing immediately. - task_center: coerce string booleans ("false"/"0"/"no"/...) in the schedule JSON compat layer via _coerce_legacy_bool, preventing recurring tasks from being misclassified as one-time. - webui QueuedSection: refresh the open detail drawer by execution id so the detail keeps updating after pagination/filter changes, independent of the current page's tasks list. - tests: add compat tests for the two task_center regressions above. Made-with: Cursor
1 parent 7dbd454 commit b6d1d42

3 files changed

Lines changed: 80 additions & 12 deletions

File tree

flocks/tool/task/task_center.py

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,25 @@
2121
log = Log.create(service="task.tools")
2222

2323

24+
_TRUTHY_STRINGS = {"true", "1", "yes", "y", "on"}
25+
_FALSY_STRINGS = {"false", "0", "no", "n", "off", ""}
26+
27+
28+
def _coerce_legacy_bool(value: object, *, default: bool = False) -> bool:
29+
"""Coerce values that may arrive as strings from legacy clients."""
30+
if isinstance(value, bool):
31+
return value
32+
if isinstance(value, (int, float)):
33+
return bool(value)
34+
if isinstance(value, str):
35+
normalized = value.strip().lower()
36+
if normalized in _TRUTHY_STRINGS:
37+
return True
38+
if normalized in _FALSY_STRINGS:
39+
return False
40+
return default
41+
42+
2443
def _normalize_task_create_inputs(
2544
type_value: Optional[str],
2645
schedule_type: Optional[str],
@@ -63,15 +82,19 @@ def _normalize_task_create_inputs(
6382
)
6483
timezone = str(schedule_data.get("timezone") or timezone)
6584
if schedule_data.get("run_once") is not None:
66-
run_once = bool(schedule_data.get("run_once"))
85+
run_once = _coerce_legacy_bool(schedule_data.get("run_once"), default=run_once)
6786
elif schedule_data.get("runOnce") is not None:
68-
run_once = bool(schedule_data.get("runOnce"))
87+
run_once = _coerce_legacy_bool(schedule_data.get("runOnce"), default=run_once)
6988

7089
if type_value:
7190
return type_value, run_once, run_at, cron, cron_description, timezone
7291
if not schedule_type:
73-
inferred_type = "scheduled" if (cron or run_at) else "queued"
74-
return inferred_type, run_once, run_at, cron, cron_description, timezone
92+
# When the caller signalled a scheduled intent (run_once / run_at / cron),
93+
# keep it scheduled so build_schedule can surface proper validation errors
94+
# instead of silently falling back to an immediate queued execution.
95+
if cron or run_at or run_once:
96+
return "scheduled", run_once, run_at, cron, cron_description, timezone
97+
return "queued", run_once, run_at, cron, cron_description, timezone
7598

7699
normalized = schedule_type.strip().lower()
77100
if normalized == "queued":

tests/tool/test_task_center_compat.py

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,51 @@ async def test_task_update_defaults_to_update_and_accepts_schedule_fields(self):
172172
assert updated.trigger.timezone == "UTC"
173173
assert updated.source.user_prompt == "更新后的执行内容"
174174

175+
@pytest.mark.asyncio
176+
async def test_task_create_rejects_run_once_without_time_instead_of_immediate(self):
177+
"""run_once=True with no run_at/cron must NOT silently become an immediate task.
178+
179+
Previously such inputs were inferred as ``queued`` and executed right away,
180+
masking missing-schedule mistakes from legacy clients.
181+
"""
182+
result = await ToolRegistry.execute(
183+
"task_create",
184+
ctx=_make_ctx(),
185+
title="缺少时间参数",
186+
description="只传了 run_once=True 但没给 run_at/cron",
187+
run_once=True,
188+
user_prompt="不应该被立即执行",
189+
)
190+
191+
assert result.success is False
192+
assert result.error is not None
193+
assert "run_at" in result.error or "cron" in result.error
194+
195+
_, total = await TaskManager.list_schedulers(limit=10)
196+
assert total == 0
197+
198+
@pytest.mark.asyncio
199+
async def test_task_create_schedule_json_accepts_string_boolean_run_once(self):
200+
"""Legacy clients may serialise run_once as the string "false"/"0" —
201+
those must be coerced to False, not treated as truthy."""
202+
result = await ToolRegistry.execute(
203+
"task_create",
204+
ctx=_make_ctx(),
205+
title="字符串布尔值兼容",
206+
description="run_once 以字符串 'false' 传入",
207+
schedule='{"cron": "*/5 * * * *", "run_once": "false"}',
208+
user_prompt="循环任务",
209+
)
210+
211+
assert result.success is True
212+
213+
schedulers, total = await TaskManager.list_schedulers(limit=10)
214+
assert total == 1
215+
scheduler = schedulers[0]
216+
assert scheduler.mode == SchedulerMode.CRON
217+
assert scheduler.trigger.cron == "*/5 * * * *"
218+
assert scheduler.trigger.run_immediately is False
219+
175220
@pytest.mark.asyncio
176221
async def test_task_update_can_disable_and_enable_scheduled_task(self):
177222
scheduler = await TaskManager.create_scheduler(

webui/src/pages/Task/QueuedSection.tsx

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -83,21 +83,22 @@ export default function QueuedSection({ onRefreshGlobal }: { onRefreshGlobal: ()
8383

8484
// When the user selects a task, also fetch it directly to be sure
8585
// we have the latest data even if it's not on the current page.
86-
const fetchDetailTask = useCallback(async (task: TaskExecution) => {
86+
const fetchDetailTask = useCallback(async (taskId: string) => {
8787
try {
88-
const res = await taskAPI.getExecution(task.id);
88+
const res = await taskAPI.getExecution(taskId);
8989
const detail = await markViewedIfNeeded(res.data);
9090
setDetailTask(detail);
9191
} catch { /* ignore — list sync will cover it */ }
9292
}, [markViewedIfNeeded]);
9393

94+
// The detail drawer is intentionally kept open across page/filter changes,
95+
// so we must refresh it by ID rather than looking it up in the current page.
9496
const refreshWithDetail = useCallback(() => {
9597
refresh();
9698
if (selectedId) {
97-
const selected = tasks.find(t => t.id === selectedId);
98-
if (selected) fetchDetailTask(selected);
99+
fetchDetailTask(selectedId);
99100
}
100-
}, [refresh, selectedId, fetchDetailTask, tasks]);
101+
}, [refresh, selectedId, fetchDetailTask]);
101102

102103
const closeDetail = useCallback(() => {
103104
setSelectedId(null);
@@ -111,7 +112,7 @@ export default function QueuedSection({ onRefreshGlobal }: { onRefreshGlobal: ()
111112
}
112113
setSelectedId(task.id);
113114
setDetailTask(task);
114-
fetchDetailTask(task);
115+
fetchDetailTask(task.id);
115116
}, [selectedId, closeDetail, fetchDetailTask]);
116117

117118
const handleAction = async (action: string, taskId: string) => {
@@ -138,8 +139,7 @@ export default function QueuedSection({ onRefreshGlobal }: { onRefreshGlobal: ()
138139
}
139140
refresh();
140141
if (selectedId === taskId) {
141-
const selected = tasks.find(t => t.id === taskId);
142-
if (selected) fetchDetailTask(selected);
142+
fetchDetailTask(taskId);
143143
}
144144
} catch (err: unknown) {
145145
toast.error(t('queued.actionFailed'), err instanceof Error ? err.message : String(err));

0 commit comments

Comments
 (0)