mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-08-12 07:49:00 +00:00
4 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Vanzeren
|
095092418c
|
fix(gateway):unify thread id validation (#4589)
* fix(gateway): unify thread ID validation at the API boundary
Thread ID entry points accepted arbitrary strings while downstream
consumers (filesystem paths, Kubernetes Provisioner, JSONL event store)
each enforced different character restrictions, so invalid IDs were
persisted first and only failed later during sandbox/workspace init.
Centralize validation in deerflow.utils.thread_id (pattern
^[A-Za-z0-9_-]{1,64}$): validate at routers, RunCreateRequest,
scheduler dispatch, paths.py, JSONL store, embedded client, and align
the Provisioner pattern (pinned by a parity test). UUIDs are still
generated only when no ID is supplied; caller-supplied opaque IDs stay
supported.
Deliberate exceptions: DELETE /threads/{id} keeps str as the legacy
cleanup escape hatch (filesystem cleanup guarded), read-only
client.get_thread stays unvalidated, and scheduler rows with legacy
invalid IDs record a failed dispatch instead of raising out of the
poll loop.
* docs: document canonical thread ID contract
README: caller-supplied thread IDs need not be UUIDs; the canonical
pattern and per-endpoint behavior. AGENTS.md: the shared
deerflow.utils.thread_id contract, its enforcement boundaries, and the
legacy-ID escape hatches.
* fix(gateway): close thread ID validation gaps at remaining entry points
Follow-up to the canonical thread ID contract: a full audit found the
uniform-422 coverage only reached about half of the thread_id surfaces.
- routers: 18 routes still took a bare thread_id: str — 13 in
thread_runs.py (including the five messages/events/workspace-changes
reads that returned 500 on the JSONL event store vs 404/empty on the
DB store), 4 read routes in threads.py, and the suggestions route
flagged in review. DELETE /api/threads/{id} keeps str as the declared
legacy-cleanup escape hatch.
- client: upload_files/delete_upload/list_uploads/get_artifact now
validate up front, fulfilling the RFC's 'all mutating entry points'
clause (get_thread stays unvalidated as the declared legacy read path).
- tui: the /resume literal-ref fallback validates against the canonical
contract and reports a descriptive error instead of failing deep in
the client.
- scripts/support_bundle.py: replace the drifted dot-allowing pattern
with a byte-identical copy of THREAD_ID_PATTERN (kept local so the
script still runs with a broken venv).
* test(gateway): guard the canonical thread ID contract against regressions
- test_thread_id_route_contract.py: static AST sweep asserting every
route handler with a thread_id parameter annotates ThreadId
(whitelist: the DELETE escape hatch), plus a runtime sweep hitting
all 44 thread_id routes with a non-canonical ID and asserting a 422
that names thread_id, plus a websocket upgrade-rejection case.
- test_thread_id_validation.py: client entry-point validation,
support_bundle pattern parity, and TUI literal-ref fallback tests.
- Align two tests that encoded the old contract (dotted IDs).
|
||
|
now-ing
|
f37c734406
|
fix(scheduler): retain launched run when post-launch bookkeeping fails (#4452) (#4504)
* fix(scheduler): retain launched run when post-launch bookkeeping fails `dispatch_task()` created a `queued` task-run row, then `_launch_run()` returned a live `run_id`, and only afterward did the queued->running bookkeeping (`update_status` + `update_after_launch`) run. When that bookkeeping raised on a transient DB error, the `except` handler marked the task-run `failed` with `last_run_id=None`. Because `failed` is outside the partial unique index `uq_scheduled_task_run_active`, this released the task's single active slot: the next dispatch cycle could no longer see the still-live run and launched a duplicate. The launched `run_id` was also dropped, breaking later recovery / reconciliation / cancellation. Track `launched_run_id` / `launched_thread_id`, set only after `_launch_run` returns. In the `except` handler: - If launch already succeeded, keep the task-run row `running` (so it keeps holding the active slot and no duplicate launch can occur) and persist the launched `run_id` on the parent task for retention. The bookkeeping retries are best-effort with logging; if they fail too the row stays `queued`, which is still active and still holds the slot, so we still report the run as launched. - If launch itself failed (no live run was created), behave as before: mark the task-run `failed` and release the active slot. The overlap-skip branch is now guarded by `launched_run_id is None` so a run that already launched can never be reclassified as a skip / failed. Adds a stateful regression test (`test_post_launch_bookkeeping_failure_does_not_release_active_slot`) that injects a failure on the queued->running write and asserts a second dispatch does not launch another run (`launch_count` stays 1) while the first `run_id` is retained on the task-run row. The test is verified to fail on `main` and pass with this change. A complement test pins the pre-launch-failure path (launch itself raises) to ensure the slot is still released when no live run exists. Fixes #4452 * style(scheduler): apply ruff format to fix lint-backend CI Reformat the two files touched by the previous commit with `ruff format` (line-length=240 config joins the hand-wrapped condition/log lines). No semantic change. Fixes the `lint-backend` CI failure on PR #4504. Co-Authored-By: Claude <noreply@anthropic.com> * fix(scheduler): key retention on launch_succeeded flag The previous invariant keyed the retention branch off `launched_run_id is not None`, but the assignment `launched_run_id = result["run_id"]` is itself post-launch code that can raise (KeyError/TypeError on a malformed _launch_run result). In that case launched_run_id stays None and the dispatch falls through to the pre-launch generic-failure path, marking the task-run row failed and releasing the active slot -- even though a live run was just created (same class of bug as #4452, narrower trigger). Flip a `launch_succeeded` flag immediately after `await _launch_run(...)` returns, before any further code that can raise, and key both the overlap-conflict guard and the retention branch off that flag. Add a regression test with a malformed launch result (missing run_id): the dispatch reports outcome="launched", the row stays running, and a second dispatch does not launch a duplicate. Addresses willem-bd review point 1 on #4504. Co-Authored-By: Claude <noreply@anthropic.com> * fix(scheduler): don't surface bookkeeping transient as task last_error In the post-launch retention path the parent task's last_error was set to the bookkeeping exception -- an infrastructure-level transient, not a run-level failure. Between the failed bookkeeping write and the run completing, the task list showed an error on a task whose run was actively running. Clear last_error (like the success path's clear-on-launch model): the run's real terminal outcome is written by handle_run_completion, and the transient itself is already recorded via logger.exception. Assert in the retention regression test that the parent task update carries last_error=None. Addresses willem-bd review point 2 on #4504 (taking the drop option). Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: now-ing <24534365+now-ing@users.noreply.github.com> Co-authored-by: now-ing <now-ing@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Daoyuan Li
|
ca3e510b7d
|
fix(scheduler): close duplicate dispatch race (#4105)
Enforce one queued or running scheduled-task run per task with a partial unique index. The migration resolves legacy duplicates before creating the index, and losing inserts use the existing conflict or skip outcomes. |
||
|
Xinmin Zeng
|
4fc08b4f15
|
feat: add scheduled tasks MVP (#3898)
* feat: add scheduled tasks MVP
* fix: harden scheduled task execution semantics
* feat(scheduled-tasks): preset-driven schedule form with timezone and live preview
Replace the raw cron input with a preset Select (hourly/daily/weekly/monthly/custom)
plus structured inputs (time picker, weekday toggles, day-of-month), datetime-local
for one-time tasks, a timezone selector defaulting to the browser timezone, and a
live human-readable preview. Reuses one ScheduledTaskScheduleInput for create and
edit; backend contract unchanged; zero new deps (pure Intl + DST-safe offset helpers).
* feat(scheduled-tasks): full-page i18n + recipe templates + E2E locale pin
Localize the rest of the scheduled-tasks page (filters, detail pane, actions,
edit form, run list, enum values) via t.scheduledTasks.* in en/zh. Add four
built-in recipe templates (GitHub Trending, news digest, issue triage, weekly
report) exposed as a chip row that pre-fills title + prompt + schedule. Pin
Playwright locale to en-US so E2E selectors stay stable against i18n. No backend
change, no new deps.
* fix(scheduled-tasks): idempotent 0003 migration, update head constants, future-date once test
Merge with main surfaced three CI failures:
- 0003_scheduled_tasks create_table collided with legacy test seeds that
build from full metadata; guard with inspector.has_table so the revision
no-ops when the table already exists (0004/0005 are already idempotent via
_helpers.py).
- persistence bootstrap concurrency/regression tests pinned HEAD to main's
0002_runs_token_usage; bump to the new head 0005_scheduled_task_thread_nullable.
- once-task router test used a fixed past run_at and tripped the
must-be-in-the-future validation; use a future date.
* address review: ok-check, 502 for trigger failure, mock fields, migration filename, doc fences
- fetchThreadScheduledTasks now checks response.ok like the other fetchers.
- trigger endpoint returns 502 (not 409) when dispatch fails outright, so
clients can distinguish a real conflict from a server-side failure.
- E2E mock normalizes scheduled-task objects with context_mode/last_thread_id
and nullable thread_id, matching the backend contract the UI renders against.
- Rename 0002_scheduled_tasks.py -> 0003_scheduled_tasks.py to match its
revision id (file was renamed in spirit already; filename now follows).
- CONFIGURATION.md: close the Tool Groups yaml fence and drop the stray fence
after the Scheduler notes so the sections render correctly.
* fix(scheduled-tasks): harden lease, poller, config, and frontend UX after review
* fix(scheduled-tasks): harden run lifecycle, overlap skip, non_interactive gating, and DST conversion after review
- defer a once task's terminal status to the run-completion hook; the task
stays running until the real outcome, and a startup sweep cancels once
tasks orphaned by a crash (launch-time 'completed' could stick forever)
- record interrupted runs as a distinct 'interrupted' run status with a
readable message; an interrupted once task ends 'cancelled', not 'failed'
- enforce overlap_policy=skip for fresh_thread_per_run via an active-run
pre-check (same-thread ConflictError can never fire across fresh threads)
- protect terminal run statuses from the late launch-path 'running' write
- honor context.non_interactive only for internally-authenticated callers;
arbitrary clients can no longer strip ask_clarification
- fix DST-stale timezone offset in zonedLocalToUtcIso by re-deriving the
offset at the resolved instant (once tasks fired an hour late around
spring-forward and the create->edit round-trip diverged)
- drop dead ScheduledTaskRunRepository.update_by_run_id; share one Gateway
API error helper between channels and scheduled-tasks frontends
* fix(scheduled-tasks): close review round-3 gaps in guards, concurrency, and API ergonomics
- scrub internal-only context keys (non_interactive) from the assembled run
config for non-internal callers: gating body.context alone left the same
key smuggle-able through the free-form body.config copied verbatim by
build_run_config
- guard update_after_launch with protect_terminal so the launch bookkeeping
write cannot clobber a once task already finalized by a fast-failing run's
completion hook (parent-row sibling of the run-row guard)
- reject a manual trigger while the task has an active run (409) instead of
launching a duplicate concurrent run on fresh_thread_per_run
- re-arm a terminal once task to enabled when PATCH pushes run_at into the
future; previously the endpoint returned 200 with a next_run_at that could
never be claimed
- make max_concurrent_runs a real global cap: each poll claims only into the
remaining budget of active (queued/running) scheduled runs
- paginate GET /scheduled-tasks/{id}/runs (limit<=200, offset) and push the
thread filter of /threads/{id}/scheduled-tasks into SQL
- stamp context.user_id on scheduler-launched runs, matching IM channels, so
user-scoped guardrail providers see the owning user
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
|