Replaces the pre-hexagonal scheduled-task implementation with a slice
built to the layering spec: a pure domain (two aggregates, two state
machines, the policy value object), output ports it declares itself,
SQL/launcher/thread adapters implementing them under `app/adapters/`,
and a composition root that is the one place any of them is
instantiated.
The old implementation mixed all of that into `app/scheduler/service.py`
and a router that reached straight into repositories, so the rules that
matter -- overlap policy, lease handling, which write owns which
timestamp -- were only reachable through a live database. They are now
unit-assertable on in-memory fakes, with the contract suite running each
port against both the fake and real sqlite, and the concurrency
invariants pinned by dedicated race tests.
Two bugs the old shape hid are fixed on the way: a completion hook that
replayed a stale snapshot and rolled back the launch write, and a
corrupt stored row surfacing to the client as a 4xx.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Introduces the ports-and-adapters standard new backend modules are
expected to follow, plus the two pieces that make it more than prose:
`deerflow/domain/` as the inner-ring namespace, and an AST test that
fails when anything under it imports infrastructure.
The spec is normative rather than descriptive -- the existing modules
predate it, so the test guards the namespace, not the whole backend.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(persistence): support custom postgres schema
* fix(persistence): address CI lint/test failures and review feedback
- Map missing psycopg import to actionable POSTGRES_INSTALL guidance in
sync/async schema-creation helpers
- Accept SQLAlchemy compound DSN schemes (postgresql+asyncpg) when
injecting search_path, normalizing to a libpq-consumable DSN
- Guard keyword-DSN tests with importorskip so they skip without psycopg
- Set database=None in sync checkpointer none-fix test to avoid MagicMock
backend resolution
- Apply ruff import sort and format
* fix(persistence): address pg-schema review feedback
- Restrict postgres_schema regex to lowercase-only so the quoted CREATE
SCHEMA matches the unquoted search_path (PG case-folds it), fixing the
mixed-case bug where tables silently fell back to public.
- Replace shlex.join/split with libpq-correct backslash escaping for the
options parameter so values containing spaces survive intact.
- Add normalize_libpq_dsn() and route the async checkpointer pool through
dsn_with_search_path() so a +asyncpg suffix is stripped and existing DSN
options (e.g. statement_timeout) are merged instead of overridden.
- Extract shared ensure_postgres_schema()/ensure_postgres_schema_async()
helpers (mapping missing psycopg to the install hint) used by all four
provider sites.
- Tests: reject mixed-case schemas, preserve space-containing libpq option,
cover normalize_libpq_dsn, and assert pool search_path via DSN.
* fix(persistence): align pg-schema test with merged store API
The main merge moved the sync Store factory to the single-path
_resolve_store_config/_sync_store_cm design, dropping the PR's
_sync_store_from_database helper. The integration test still imported
the removed symbol, breaking test collection (backend-unit-tests).
Resolve the store config from a DatabaseConfig and drive it through
_sync_store_cm instead.
* fix(persistence): address pg-schema review feedback
- reject trailing/leading whitespace in postgres_schema via re.fullmatch
(a $-anchored re.match let "deerflow\n" through, silently landing tables
in public)
- re-escape all whitespace (TAB/CR/LF) when re-joining libpq options so a
caller's pre-existing options value round-trips losslessly
- re-validate the identifier inside create_schema_sql as defense-in-depth
at the SQL-emitting boundary
- accept the postgres:// short scheme in the alembic search_path injection
- close the sync psycopg connection explicitly (psycopg3 __exit__ does not
close()), mirroring the async path
- drop the partial checkpointer/store reset on a database config change;
database is restart-required and the ORM engine is not rebuilt, so a
partial reset would half-migrate the deployment
* docs(config): complete the postgres_schema migration checklist
Address PR review (P1): the documented `public`->schema migration only
moved runs, run_events, threads_meta, feedback, and users. That strands
every other DeerFlow-owned table -- the four channel_* tables, both
scheduled_* tables, agents, and (critically) alembic_version -- in
`public`. On restart bootstrap treats the partially-populated target
schema as unversioned, re-baselines it, and replays migrations while the
real rows stay invisible in `public`.
List the full owned set explicitly, call out alembic_version as required,
and keep the "discover the rest" query for version-drift safety.
* refactor(checkpointer): drop test-only _sync_checkpointer_from_database
Address PR review: the helper was only reached by the env-gated
integration test and re-implemented the DatabaseConfig->CheckpointerConfig
backend resolution that _resolve_checkpointer_config already owns, so a
future backend added there would silently miss this path. Mirror the store
side of the same test, which reuses the production path directly:
_resolve_checkpointer_config(...) + _sync_checkpointer_cm(...).
* fix(sandbox): claim ownership before readiness-timeout destroy (#4248)
When a freshly-created sandbox fails to become ready within the 60s timeout, _create_sandbox / _create_sandbox_async destroyed the container with a bare self._backend.destroy(info) call. Ownership is published by _register_created_sandbox only after the readiness gate, so for the whole timeout window the container ran unowned — exactly the state a peer gateway startup reconciliation is built to adopt across. With the claim skipped, a peer could adopt the not-yet-ready Pod and this instance subsequent stop landed on the turn the peer had just handed it: a cross-instance kill of an active turn.
Route the destroy through a new _destroy_unready_sandbox helper that first claims the teardown lease (claim(..., for_destroy=True) writes the del: marker) and holds it via _held_teardown_lease for the duration of the stop, matching the ownership guard every other reap path already uses (_destroy_warm_entry, _drop_unhealthy_reserved). Fail closed if a peer already holds the lease or the ownership store cannot answer: leave the container for the peer own reconciliation rather than stopping it from underneath an active turn.
Fixes#4248.
* fix(sandbox): reserve local teardown around readiness-timeout destroy
Review feedback (AnnaSuSu) on #4505: the ownership claim is only the
cross-instance half of the guard — claim() succeeds against our own
lease by design, so in the window between the readiness timeout and the
claim a same-process _reconcile_orphans (idle checker, every 60s) can
adopt the unready container into _warm_pool; the subsequent claim still
succeeds and the stop lands on an entry this instance has just adopted,
leaving a dead warm entry for the next reclaim to hand out.
Wrap the still-untracked check, claim, and stop in
_reserve_local_teardown / _finish_local_teardown, with the predicate
checking the id is absent from the active and warm maps — the same
pairing _destroy_warm_entry already uses.
Add an interleaving regression test mirroring
test_reconcile_does_not_adopt_a_container_this_instance_is_tearing_down:
reconcile runs while the destroy thread is parked after reserving but
before its `del:` claim lands, and must not adopt. A mirror test
asserts a genuinely unowned container is still adopted when no teardown
is in flight, so the reservation cannot over-block reconciliation.
* style(sandbox): apply ruff format to readiness-timeout destroy changes
---------
Co-authored-by: now-ing <24534365+now-ing@users.noreply.github.com>
* 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>
The console cost estimator summed per-run spend across every priced model
without checking that they shared a currency, so a deployment that priced
one model in CNY and another in USD produced a meaningless aggregate under
a single currency label. Track the first priced currency and, on a
mismatch, log a warning and drop all pricing so cost/currency fields report
null instead of an invalid sum. Currency codes are now trimmed and
case-normalized so equivalent spellings don't false-trip the guard.
* fix(skills): offload update_skill's blocking IO and share the config write lock
Re-applied on top of main rather than merged: update_skill has since gained
admin gating, user-scoped storage and a PUBLIC vs CUSTOM/LEGACY split, so the
offload is applied per path.
- PUBLIC: the extensions_config.json read-modify-write (path resolve, snapshot,
merge, write, reload) moves to a worker thread via asyncio.to_thread. The
payload is built from a snapshot so the cached singleton is never mutated in
place while the write is still in flight.
- CUSTOM/LEGACY: set_skill_enabled_state is offloaded; the non-user-scoped
fallback takes the same shared-file RMW path as PUBLIC.
- Both load_skills calls (and storage construction) are offloaded.
The RMW lock now lives next to reload_extensions_config as
get_extensions_config_write_lock() and is acquired by both the skills router and
the MCP router, which performs the same read-modify-write on the same file.
Previously each router held its own module-local lock, so once both sides
offloaded, a PUT /api/mcp/config could run inside a skill toggle's read->write
window and the later write would silently drop the other's change.
The lock is keyed by the running event loop rather than being a module-level
singleton: asyncio primitives bind to the first loop that awaits them, which
makes a plain module-level lock unusable in a process that runs more than one
loop.
Adds a cross-router regression anchor asserting a skill toggle and an MCP update
never overlap inside the RMW (max in-flight 1); it observes 2 when the routers
use separate locks.
* fix(config): own the extensions_config RMW lock from the worker thread
An asyncio.Lock held around `await asyncio.to_thread(...)` protects only the
awaiting task. If that task is cancelled the context manager releases the lock
immediately while Python keeps running the worker thread, so a second skills or
MCP writer could acquire it and operate on extensions_config.json concurrently
with the first worker — reopening the lost-update window this was meant to close.
The per-event-loop keying had a second hole: writers on different loops got
different locks and so did not exclude each other at all.
Replace it with a process-wide threading.Lock acquired *inside* the worker that
performs the RMW (`_write_extensions_skill_state` and `_apply_mcp_config_update`),
so ownership belongs to the thread doing the writing and is held until the write
and reload actually finish, regardless of what happens to the caller. A
threading.Lock also has no event-loop affinity.
Adds a cancellation regression: the skills worker is paused inside the lock, its
route task is cancelled, and the MCP writer is started — the MCP RMW must not
enter until the skills worker is released. Against the previous asyncio-lock
design this test fails with ['skills-enter', 'mcp-enter'].
The two existing serialization tests instrumented by replacing the worker
functions, which now bypasses the lock under test; they instead patch inside the
real workers (each module's reload_extensions_config, the last step under the
lock) so the production lock is exercised.
---------
Co-authored-by: ly-wang19 <ly-wang19@users.noreply.github.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* fix(runs): show run duration once per run and label it as elapsed work
turn_duration is the run's wall-clock lifetime (updated_at - created_at),
but both message endpoints stamped it onto every AI message of the run
and the frontend rendered each stamp as 'Thought for X seconds' — the
same number repeated per message, and tool-wait time presented as model
thinking latency (#4152).
- share one stamping helper between list_messages and list_run_messages:
only the run's final non-middleware AI message carries turn_duration
(list_run_messages already tried to do this but iterated reversed()
without stopping at the first match)
- completed-state trigger copy becomes 'Worked for X seconds' since the
number includes tool execution, not just thinking
Fixes#4152
* fix(threads): stamp turn_duration once per run in /history replay too
get_thread_history stamped turn_duration on every AI message of a run
instead of only the last one, so a run with several AI messages (e.g.
a tool call followed by a final answer) showed the same duration badge
more than once on reload.
Rebasing onto main picked up #4118, which replaced the /history
duration path with a checkpoint-metadata fast path plus an
event-store/run-manager fallback for legacy checkpoints - both of
which stamped every AI message per run_id, reintroducing this same
bug on both paths. This commit folds the once-per-run stamping
(stamp_turn_duration_on_last_ai) into both paths instead of only the
legacy one, and narrows the fast-path/fallback boundary to a per-run
completeness check (turn_run_ids - checkpoint_run_durations) instead
of a per-message one, so a run whose duration is already in checkpoint
metadata never re-triggers the event-store correlation fallback just
because its non-final AI messages correctly have no turn_duration.
* fix(frontend): stop caching a client-measured turn_duration per message
A run can produce several standalone content-only AI messages (e.g. a
subagent handoff), each briefly becoming the newest message and each
mounting its own Reasoning timer via the shared turnStartTime. Wiring
onTurnDurationChange let that per-message timer cache and display a
"Worked for X seconds" badge the moment a later message superseded it
- with a different, premature number, since the timer only measured
that message's own streaming window, not the run's total elapsed
time. That reintroduced the #4152 duplicate-badge bug through a path
the backend fix (once-per-run stamping) doesn't cover.
cachedDuration now only ever mirrors a backend-confirmed
turn_duration (already handled by the other effect here), so a
superseded message can no longer display a stale or wrong duration.
* fix(frontend): use 'Worked for' framing even without a persisted duration
The completed-state copy diverged depending on whether turn_duration
had landed yet: "Thought for a few seconds" (no duration) vs. "Worked
for X seconds" (duration present). Both describe the same completed
run, just with or without a persisted number, so the framing
shouldn't disagree on what happened.
* docs(runs): note why the middleware skip is inert on /history, add test
stamp_turn_duration_on_last_ai's middleware-caller skip only has an
effect on the event-store message shape (metadata.caller); checkpoint
messages replayed on /history never carry that field. Document why
that's not a gap - middleware writes go to thread metadata, not the
messages channel, so no middleware message ever reaches a
checkpoint's messages list.
Also lock the middleware-skip contract on list_thread_messages with
its own regression test, mirroring the existing one for
list_run_messages - the thread feed only gained this skip through the
same shared helper, and previously stamped every AI message,
middleware included.
* chore(frontend): retain current run duration UI
The frontend behavior was superseded by #4348; keep the latest main implementation while retaining this PR's backend fix.
* test(frontend): retain current reasoning coverage
Align the old PR test with the frontend implementation already merged in #4348.
---------
Co-authored-by: fancyboi999 <fancyboi999@users.noreply.github.com>
* fix(sandbox): unwrap Overwrite-wrapped state in ensure_sandbox_initialized
The same fork-restored wrapper that crashed after_agent also reaches the sandbox init path, where sandbox_state.get() on the Overwrite object raises AttributeError. Share the unwrap helper from #4381's follow-up module deerflow/sandbox/overwrite.py and apply it at both init sites.
Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com>
* fix(sandbox): note why discarding fork_restored at the reuse sites is safe
* fix(sandbox): unify the Overwrite unwrap helper and pin the fall-through
- middleware.py now imports unwrap_sandbox from overwrite.py instead of
keeping a second local copy whose docstring had already drifted; the
shared helper covers both crash forms (subscript TypeError and the
.get()-form AttributeError)
- test the acquire fall-through: when the fork-restored id is gone from
the provider, a fresh sandbox is acquired and the stale wrapped state
is replaced by the plain acquired dict
- the reuse-path test now also asserts runtime.state["sandbox"] stays
wrapped, pinning the don't-treat-as-owned contract after_agent relies on
Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com>
* fix(sandbox): unwrap Overwrite state in the sibling sandbox readers
Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com>
---------
Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
A browser client served from a different origin than the Gateway never
learns the id of the run it just created, so a brand-new thread keeps its
placeholder route for the whole session and every action gated on an
established thread — edit and rerun, regenerate, branch — stays hidden
until the page is reloaded.
Run-creating routes return the run's id in `Content-Location`, and the
LangGraph SDK resolves run metadata from that header alone. It is not
CORS-safelisted, so a cross-origin response hides it from JS unless the
server lists it in `Access-Control-Expose-Headers`. `useStream`'s
`onCreated` therefore never fires and the app cannot rewrite its route.
Expose it. `GATEWAY_CORS_ORIGINS` is a supported deployment mode, so the
CORS middleware has to carry everything that mode needs to read. Same-origin
nginx deployments are unaffected because CORS never applies to them.
* feat(checkpoint): make delta snapshot_frequency configurable
* fix(config): carry legacy checkpoint_delta_snapshot_frequency with warning
Addresses review on #4516: the rename from the flat
database.checkpoint_delta_snapshot_frequency key to nested
database.checkpoint_delta.snapshot_frequency silently dropped the old
value (pydantic extra="ignore"). Add a before-validator that maps the
legacy key onto the nested one with a deprecation warning (nested key
wins when both are set), plus a CHANGELOG breaking-change note covering
the rename and the 1000 -> 10 default change.
* fix(checkpoint): validate frozen snapshot frequency
* feat(lark): sidecar credential broker for sandbox lark-cli (Pattern B)
Removes the plaintext Lark credential mounts (appSecret + OAuth tokens)
from the sandbox container. A long-running broker sidecar owns lark-cli
and the per-user config/data dirs and serves the command surface over
Pod loopback; the sandbox gets only a forwarding shim on PATH, so the
raw credential files never exist in the sandbox filesystem.
- lark_broker.py: stdlib-only loopback broker (argv passthrough with
shell=False, server-injected credential env, bounded I/O) + shim
script constant + install-shim mode.
- docker/lark-cli-broker: init(install-shim) + serve image.
- provisioner: LARK_CLI_BROKER_IMAGE + provision_lark_cli_broker →
shim init container + lark-cli-broker sidecar (config/data mounted
sidecar-only); credentials dropped from the sandbox container;
/api/capabilities reports lark_cli_broker_image. Broker supersedes
the Pattern A init-container binary when both are configured.
- gateway: lark_cli_env_overlay(broker=True) omits config/data env;
sandbox_lark_broker_active() TTL-cached mode resolver; broker added
to sandbox_runtime_mode / readiness and the settings UI.
Opt-in and off by default (empty LARK_CLI_BROKER_IMAGE ⇒ no change).
Closes#4338
* fix(lark): address Pattern B broker review findings (#4501)
Follow-up to the sidecar credential broker addressing the PR #4501 review:
- shim: split the on-PATH lark-cli into a /bin/sh launcher + Python shim body
so broker mode fails loudly (exit 127, actionable message) instead of ENOEXEC
when the sandbox image ships no python3; interpreter pinnable via
DEERFLOW_LARK_BROKER_PYTHON. Launcher bakes in the shim's absolute path since
$0 is the bare command name when run off PATH.
- broker: drop the dead cwd payload field (broker can't see the sandbox FS) and
document the command-surface-only / no-file-IO limitation.
- broker: return a structured 500 JSON on unexpected exec errors so the shim
gets a meaningful message, not an opaque transport failure; set a handler
socket timeout to bound slow/stuck connections.
- broker: add an opt-in DEERFLOW_LARK_BROKER_DENY_SUBCOMMANDS denylist that
refuses secret-dumping subcommands before spawning the binary, forwarded from
the provisioner sidecar.
- gateway: tighten the per-bash-call broker probe timeout (1.5s) and cache
negatives longer (300s) so non-broker remote-provisioner users don't pay a
latency hit; guard the mode cache with a lock; drop the dead
_probe_provisioner_lark_cli_init_image wrapper.
- docs: remove the broken design-doc link from the broker README.
Adds tests for launcher python resolution, cwd omission, denylist enforcement,
500-on-error, hot-path probe timeout + negative caching, and provisioner
denylist-env wiring.
Editing the only turn of a thread reran the original prompt: the model
answered the question the edit was replacing while the UI showed the
edited text, and the edit vanished on reload.
The replay-base lookup decided whether a checkpoint predates the target
user message by message id alone. DynamicContextMiddleware re-keys the
first user turn to `{id}__user` mid-run, so every checkpoint written
before it holds the same prompt under an id the lookup cannot match. The
scan walked past those and anchored inside the run that produced the
turn — a checkpoint that still contains the original prompt and owns the
injection node's pending writes, which the replay then re-added after the
edited message.
Require the replay base to be a settled checkpoint (no pending tasks) in
both the lineage walk and the chronological fallback. That rule is
middleware agnostic: the first turn now anchors on the thread's empty
initial checkpoint and later turns on the previous run's tail, which also
drops the existing reliance on LangGraph discarding a stale `__start__`
write.
Edit replay additionally passes `head_checkpoint` so it resolves its base
lineage-first like regenerate does, and a replayed user message is
restored to its pre-swap id: replaying `{id}__user` into a state that has
no reminder yet makes the middleware treat the turn as already injected
and silently drops its date and memory block.
Frontend: a prepared replay masks the turn it supersedes, so the
optimistic-message baseline is taken from the post-mask human count. The
pre-mask count can never be exceeded when the replay puts exactly one
human message back, and on the first turn the runtime re-keys the
replacement message so identity comparison cannot stand in for the count.
Fixes#4531
Renaming a conversation and then editing one of its turns reverts the
title to whatever it was before that turn ran, so the user's own name for
the thread is silently replaced by an older automatically generated one.
Edit replay resumes from the checkpoint before the edited turn, and that
checkpoint predates the rename. Regenerate already guards against exactly
this rollback by replaying the current title as graph input; the edit
replay path was added later and did not carry the guard over.
Replay the title the same way, but only when the replay base already has
one. An untitled base belongs to a thread the title middleware has not
named yet — pinning the current title there would keep a name generated
from the prompt this edit just replaced, and stop the middleware from
naming the rewritten turn.
`browserless` reads `cfg["timeout_s"]`, while its sibling web providers
`crawl4ai` and `jina_ai` read `cfg["timeout"]`. Tool configs allow extra
fields, so the unrecognised spelling is dropped without a diagnostic: someone
adapting one provider's config snippet for another silently gets the 30s
default instead of the timeout they set. (Observed in the other direction, on a
deployment whose crawl4ai entry carried `timeout_s`.)
Accept both keys, preferring the documented `timeout_s` when both are present.
While adding coverage, two pre-existing bugs in the same three lines surfaced,
both already guarded in crawl4ai/jina_ai but not here:
- `timeout_s: "30s"` (or any non-numeric string) raised ValueError out of
`float(raw)` during tool construction rather than falling back.
- `timeout_s: off` -- YAML parses that as `False`, and `float(False)` is
`0.0`, so every request timed out immediately against a healthy server.
`_coerce_timeout` now mirrors the sibling providers: booleans and unparsable
strings fall back to the default, with a warning for the string case.
Tests: five cases in tests/test_browserless_client.py covering both keys, the
precedence order, and both coercion bugs. Verified red before the fix (3 of 5
fail) and green after.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* feat(memory): integrate FTS5 retrieval adapter
* deps: add jieba as default dependency for Chinese tokenization
Without jieba, FTS5 unicode61 tokenizer treats entire Chinese sentences
as single tokens, making single-character or sub-phrase searches
impossible (e.g. '吃' or '油泼面' returns 0 hits against
'用户喜欢吃油泼面'). jieba segments Chinese text into meaningful tokens
before indexing.
* fix(memory): avoid treating hyphens as FTS5 operators
* feat(memory): make Chinese tokenization optional
* fix(memory): warm every requested retrieval scope
* fix(memory): close retrieval resources on shutdown
* fix(memory): close backend when shutdown flush fails
* fix(memory): recreate corrupt retrieval index
* fix(memory): tolerate partial retrieval rebuilds
* fix(memory): warm retrieval index in background
* fix(memory): preserve shutdown flush budget
* fix(memory): stop retrying partial lazy rebuilds
* fix(memory): close retrieval through storage
* refactor(memory): simplify retrieval scope limit
* docs(memory): clarify retrieval shutdown lifecycle
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* feat(channels): share inbound webhook dedupe across pods via Postgres (#4120)
* ci: run cross-pod inbound dedupe integration tests in CI
Expose the job Postgres service via DEDUPE_TEST_POSTGRES_URL so the integration tests (issue #4120) actually execute instead of silently skipping. Normalize the URL for asyncpg (postgresql:// -> +asyncpg, drop libpq-only sslmode) and await the now-async _is_duplicate_inbound in test_github_dispatcher.
* feat(sandbox): add Tenki cloud sandbox provider
Adds deerflow.community.tenki, a SandboxProvider backed by Tenki cloud
microVMs, alongside the existing e2b_sandbox / boxlite / aio_sandbox
backends. Selected via `sandbox.use: deerflow.community.tenki:TenkiSandboxProvider`
(resolved by class path, so the change is purely additive).
The full Sandbox contract is implemented — execute_command plus
read/write/update/download_file and list_dir/glob/grep — with file ops run
as busybox-portable shell commands (cat / find / grep / chunked base64),
reusing deerflow.sandbox.search, mirroring e2b_sandbox and boxlite. Tenki's
SDK is synchronous, so unlike boxlite there is no event-loop bridge.
Tenki sandboxes run as an unprivileged user with /mnt root-owned, so the
/mnt/user-data virtual prefix is remapped under the writable home dir (like
e2b_sandbox); the provider also best-effort sudo-symlinks /mnt/user-data to
that home dir so agent shell commands using the literal path still work.
Sandboxes are pooled per (user, thread) with warm reclaim, a replica cap,
and an idle reaper via the shared WarmPoolLifecycleMixin. Transient
transport blips get one bounded retry; terminal session errors evict and
recreate.
Only the stable Tenki surface is used (create/terminate + exec/shell/fs) —
no volumes, snapshots, or template builds — so any stock base image works.
The tenki-sandbox SDK is an optional extra (deerflow-harness[tenki]) and is
imported lazily, so a default install and every other provider are
unaffected.
Tested: unit suite runs in CI without tenki-sandbox installed; a live
integration test and full-surface e2e were verified against real Tenki
sandboxes.
* fix(sandbox): remove unsafe auto-retry from Tenki exec
Pre-merge review caught that the transient-transport retry sat at the
universal _exec layer, so it retried every operation — execute_command and
base64 file-write chunks included. gRPC has no exactly-once guarantee: a
"socket closed" ack-drop after the server already ran the op means the retry
runs it twice, double-firing command side effects and duplicating a write
chunk mid-file (silent binary corruption on multi-chunk writes).
exec is not idempotent, so it must not be auto-retried. Reverts to the
boxlite/e2b behavior: a transient error surfaces to the caller (returned as
text by execute_command, raised by the file ops); a terminal session error
still evicts the sandbox so the next acquire rebuilds it. Verified live
end-to-end across 31 edge cases (empty/binary/unicode/chunk-boundary files,
shell-metachar content, error paths, list/glob/grep, warm-pool reclaim,
concurrency).
* fix(sandbox): address Tenki provider review feedback
- Use Tenki's native sandbox.fs API for all file transport (read_text,
read_bytes, write_stream, mkdir) instead of cat/chunked-base64 over shell.
Uploads stream in 1 MiB frames; append is read-modify-write because the
write stream has no append mode (same approach as community/e2b_sandbox).
- download_file streams via fs.read_stream and enforces the 100 MB cap on
bytes actually received, closing the TOCTOU window between the old
wc -c size probe and the read.
- list_dir/glob/grep report paths back under /mnt/user-data instead of the
sandbox-internal home dir, so results feed straight into the file APIs.
- Create with wait=False and await wait_ready() here: create(wait=True)
raises with the session handle still inside the SDK, leaking a running
microVM this provider could never terminate.
- Configure the sandbox lifetime (max_duration, default 4h) and expose
sticky; without it Tenki reaps a reused thread's sandbox after ~30 min.
- close() terminates before marking the adapter closed and re-raises real
failures, so a failed termination stays retryable instead of silently
leaking a billed microVM; an already-gone session still counts as closed.
- Bump the optional extra to tenki-sandbox>=0.4.0 and commit backend/uv.lock.
* fix(sandbox): scope tenki grep() glob filter to its directory prefix
Mirrors #4168, which fixed the same defect in the E2B provider. The tenki
adapter reduced a directory-scoped pattern like "src/*.js" to its basename
before filtering, so the search silently broadened to every matching-extension
file in the tree. Post-filter grep's hits through path_matches() against the
path relative to the search root, the same way glob() already does, so both
agree on what a directory-scoped pattern means.
* fix(sandbox): address Tenki provider review — eviction, id width, write lock, grep -H
Four fixes from the upstream review:
download_file no longer swallows terminal transport errors. The broad
`except OSError: raise` re-raised ConnectionError/BrokenPipeError/EOFError
(all OSError subclasses that _is_terminal_failure treats as terminal) before
_note_failure ran, so a session that died mid-download was never evicted. Only
our own EFBIG size-cap now passes through without eviction.
Sandbox id widened from 32 to 64 bits (`[:8]` to `[:16]`), matching
community/e2b_sandbox. The warm pool is keyed by this id with no full-seed
fallback, so a collision could let one user reclaim another's parked sandbox on
a multi-tenant gateway.
_fs_op now holds the lock across the op, not just the fs lookup, so concurrent
calls on the same sandbox serialise over the SDK's shared connection. The
eviction callback runs after the lock is released to avoid a lock-order
deadlock with the provider. The append read-modify-write is serialised by a
dedicated _write_lock so two concurrent appends can't clobber each other.
grep passes -H so a search whose path resolves to a single file still prints
the filename; without it the file:line:text unpack dropped every match.
* fix(sandbox): address Tenki provider review round 2
- validate config `environment` at load time (_validate_extra_env) so a bad
key fails fast instead of surfacing as an SDK error mid-command
- document the deliberate lock decision in download_file: the instance lock is
dropped before streaming so a 100 MB download can't block every other tool;
terminal transport errors still evict via _note_failure
- tighten the terminal-error comment to note ConnectionError/BrokenPipeError/
EOFError are also treated terminal via isinstance
- document TenkiSandboxProvider in backend/AGENTS.md (provider detail, warm-pool
destroy hook, community provider list)
- add a commented Tenki block to config.example.yaml for parity with AIO/BoxLite
- tests: config env validation, grep -F/case-sensitive flags, glob include_dirs,
list_dir max_depth, bootstrap-failure warning branch
* fix(sandbox): make Tenki bootstrap non-interactive and time-bounded
The create-time bootstrap runs under the per-scope acquire lock, so a hang
would stall acquire for that scope indefinitely:
- use `sudo -n` so a password-requiring sudoers entry fails fast (swallowed by
the existing `|| true`) instead of blocking on a tty password prompt
- pass a timeout to the bootstrap `remote.exec` so any other stall drops to the
existing warning path rather than wedging acquire
Best-effort by design; the file APIs still work via the home remap on failure.
* test(sandbox): pin Tenki bootstrap timeout to its actual value
Assert bootstrap["timeout"] == _BOOTSTRAP_TIMEOUT instead of `is not None`, so
a regression to timeout=0 (treated as no timeout by some SDKs) or an unrelated
value is caught rather than passing a weaker non-None check.
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* feat(dingtalk): support inbound file and image attachments
DingTalk previously dropped picture and file (document) messages because
`_on_chatbot_message` ignored any message with empty text, so users could
not send files to the agent. This adds inbound attachment support, mirroring
`FeishuChannel`:
- `_extract_files` parses `picture`/`richText` image downloadCodes and `file`
(document) descriptors. `dingtalk_stream.ChatbotMessage.from_dict` does not
parse `file` messages, so `_DingTalkMessageHandler.process` stashes the raw
callback payload on the message (`_df_raw_data`) for the document descriptor.
- `receive_file` downloads each attachment by `downloadCode` via the robot
`messageFiles/download` OpenAPI, persists it into the thread uploads bucket,
syncs it into a non-local sandbox, and prepends the sandbox virtual path to
the message text so the agent can read the file by path.
- Filenames go through the shared `uploads.normalize_filename` helper, which
strips directory components and rejects traversal patterns.
Outbound `send_file` already existed; this completes DingTalk file parity with
Feishu on the inbound side. Adds 21 tests covering extraction, download-by-code,
persistence/sandbox sync, filename sanitization, and the handler raw-data stash.
* fix(dingtalk): address inbound-file review feedback
Follow-up to the review on #4423:
- Make the fallback filename safe by construction. `download_code` is
attacker-controllable webhook data and was embedded into `fallback_name`
unsanitized; it only avoided escaping the uploads directory because the
resulting write failed with OSError. It is now restricted to
`[A-Za-z0-9_-]` before use. Covered by a test that reproduces the old
behaviour (`uploads/dingtalk_../../evil.png`) and by a test that actually
exercises the previously untested `except ValueError` branch (`".."`,
whose basename — unlike `../../etc/passwd` — does raise).
- Log the swallowed `get_image_list()` failure instead of silently returning
no images, so an SDK parse failure is distinguishable from a richText
message that genuinely has no inline images.
- Surface failed downloads to the agent as a short `[failed to load ...]`
marker rather than silently omitting the attachment, so a user whose file
did not load does not simply appear to be ignored. Keeps the cleaner text
shape while restoring the signal Feishu provides.
Tests: 119 passed (was 115).
* fix(dingtalk): claim unique upload names and refuse symlinked destinations
Round 2 review follow-up on #4423. Both findings reproduce as failing tests
against the previous head.
- Inbound attachments no longer overwrite each other. Generated names repeat
across messages (every picture message yields "image.png", richText yields
"image_0.png"), so a later attachment silently replaced an earlier one whose
virtual path had already been prepended to the message text — the agent could
read bytes that were not the ones its prompt referenced. The destination name
is now claimed with the shared `claim_unique_filename` against the live
directory contents, which also covers a real filename sent twice
(`quote.xlsx`), a case Feishu's inline naming does not handle either. The
claim and the write happen under one lock so two attachments cannot resolve
to the same free name.
- Writes go through the shared `write_upload_file_no_symlink` instead of
`Path.write_bytes`. Uploads dirs may be mounted into local sandboxes, so a
sandbox process could leave a symlink at a future upload name and redirect a
gateway-privileged write outside the bucket; the regression test shows the
old code creating the out-of-bucket target.
Tests: 123 passed (was 119).
* fix(dingtalk): harden the inbound download path (self-audit)
Proactive hardening pass over the new inbound path; each fix reproduces as a
failing test against the previous head.
- Contain token failures. `_get_access_token()` sat outside the try in
`_download_by_code`, and the manager awaits `receive_file` without one — a
DingTalk auth hiccup during a file message aborted the whole chat turn with
no reply. Token acquisition moves inside the try, and `receive_file` gains
per-attachment isolation so no unforeseen error can escape past the marker.
- Cap inbound size. The download buffered arbitrary bytes in memory
(`response.content`) with no limit, while outbound uploads already enforce
one. The body is now streamed and dropped once it exceeds
`_MAX_INBOUND_FILE_SIZE_BYTES` (50 MB), surfacing as a failed-load marker.
- Sanitize the failure marker. It embedded the raw webhook `fileName`; a
newline could forge a standalone `/mnt/user-data/uploads/...` line inside
msg.text and an over-long name bloated it. Markers now collapse whitespace
and cap at 80 chars.
- Keep blocking IO off the event loop. `ensure_thread_dirs`, the uploads-dir
resolve, sync `SandboxProvider.acquire`, and `sandbox.update_file` all ran on
the loop; directory prep now lives inside the same `asyncio.to_thread` as the
claim+write, and sandbox sync uses `acquire_async` + an offloaded
`update_file`. Locked by a strict Blockbuster anchor
(tests/blocking_io/test_dingtalk_receive_file.py), verified to fail with
`BlockingError: Blocking call to os.mkdir` when the offload is reverted.
Tests: 127 + 1 blocking-io anchor (was 123); tests/blocking_io/ suite 55 passed.
* fix(dingtalk): surface missing-sandbox sync as a failed load
Round 3 follow-up on #4423:
- When a non-local sandbox acquire succeeds but the provider cannot resolve
the instance, _receive_single_file returned the virtual path anyway — a
path the agent's sandbox cannot read. Mirror Feishu: log and return "",
so the [failed to load ...] marker fires instead. Red-first test:
test_missing_sandbox_after_acquire_yields_marker.
- Drop the dead GetResponse / FakeClient.get scaffolding left in
test_oversized_download_is_dropped from its red-first iteration.
Tests: 128 + 1 blocking-io anchor (was 127 + 1).
* fix(dingtalk): treat non-local sandbox sync failure as a failed load
Round 4 follow-up on #4423. The sync except-branch logged and still returned
the virtual path when acquire or update_file raised on a non-local sandbox —
the same handing-the-agent-an-unreadable-path failure mode the sandbox-is-None
branch was just fixed for, and exactly the leg the suite did not exercise.
Feishu's except-branch returns its failure marker; DingTalk now does the
equivalent (return "" so the failed-load marker fires). Red-first test:
test_update_file_failure_yields_marker.
Tests: 129 + 1 blocking-io anchor (was 128 + 1).
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* feat(clarification): structured form fields for human-input cards
Add a request-side v2 `form` mode to the ask_clarification protocol so
business flows (e.g. expense reimbursement) can collect several values
in one card instead of sequential free-text questions:
- `ask_clarification` gains a restricted `fields` parameter (text /
textarea / number / select / multi_select / checkbox / date)
- ClarificationMiddleware validates and normalizes fields explicitly
(whitelisted types, unknown -> text, select-likes without options ->
text, duplicate/invalid entries dropped, all-invalid falls back to
the legacy modes) since the middleware short-circuits before tool
execution; the plain-text fallback lists fields for IM channels
- Form payloads carry `version: 2` so older frontends degrade to the
text fallback; replies stay on the v1 response protocol — the card
submits a readable summary as `response_kind: "text"`, so journal
persistence and answered-card recovery are unchanged
- Frontend renders typed field controls with required-field validation
and compact multi-select chips
Part of #4400 (scope narrowed per maintainer feedback: request-side
only, no new response kinds, no top-level multi_choice).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(clarification): harden form protocol per review feedback
Address the five review points on #4406:
- Reject field names colliding with JS Object.prototype members on both
sides; frontend reads form values via own-property access only, so
`constructor`/`toString`-style names can no longer leak inherited
members into required validation or the submitted summary
- Close open requests answered through the legacy text fallback: a
visible plain human reply (no response metadata) now marks every
previously-opened request as answered, so upgrading to a v2-aware
frontend cannot leave the composer locked on an already-answered card
- Give checkbox fields deterministic boolean semantics: values are
seeded to an explicit false ("no" in the summary) and `required` means
must-agree/consent; documented in the tool schema
- Make middleware field validation atomic: structurally broken entries
(bad/duplicate/reserved names, over-cap field/option counts or text
lengths) degrade the whole form instead of silently dropping fields;
options are trimmed/deduped with blanks removed so the backend never
emits payloads the frontend parser rejects
- Associate form labels/controls (htmlFor/id), aria-required,
aria-invalid, and error descriptions for accessibility
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* refactor(clarification): type the fields item schema via TypedDict
Replace `fields: list[dict[str, Any]]` with `list[ClarificationFormField]`
(a TypedDict with `name` required and the type whitelist as a Literal) so
the provider-facing tool schema documents the item shape instead of an
opaque object relying on the docstring. Runtime validation is unchanged
and stays in ClarificationMiddleware, which intercepts the call before
tool execution. Addresses the non-blocking review suggestion on #4406.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(frontend): drop unsupported aria-invalid from multi-select group
jsx-a11y: role=group does not support aria-invalid; the error linkage
stays via aria-describedby.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(clarification): coerce numeric required flags and normalize fields once
- `_normalize_bool` now coerces 1/0 (some providers serialize booleans
as integers), so `required: 1` no longer silently flips to optional
- `_handle_clarification` normalizes `fields` once and passes the result
to both the text fallback and the payload builder
Addresses the non-blocking review nits on #4406.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(clarification): harden form protocol per contract review round 2
Backend:
- Guard unhashable JSON in the intercept path: `type: []`/`{}` degrades
the field to text and `clarification_type: []` coerces to str instead
of raising TypeError (which, with return_direct, ended the turn with
an error and no card or fallback)
- Add a total budget over the serialized normalized fields (16KB UTF-8
bytes): per-item caps alone admitted forms whose IM text fallback
exceeded channel delivery limits (Slack 40k chars, Feishu ~30KB card),
silently truncating trailing fields; a boundary test proves any
accepted form's fallback stays deliverable
Frontend:
- Submission value now appends a JSON block keyed by stable field names
(readable summary alone is delimiter-ambiguous), with a collision
regression test
- Parser boundary tightened to match backend constraints: empty option
values (Radix SelectItem crash), duplicate option ids/values,
duplicate field names, and the form<->version-2 binding are rejected
- Keep the error node mounted while any field is still invalid so
aria-describedby never points at a removed element (happy-dom
interaction test)
- Required semantics are now accessible: native checkbox control (no
HTML required attribute — it would intercept the custom submit path),
visually-hidden localized "required" markers next to the aria-hidden
asterisks
- Legacy-fallback closure narrowed to the latest unanswered request:
nothing guarantees a single outstanding clarification across runs, and
closing all would silently swallow older decisions; an older request
left open becomes the active card again
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(frontend): keep clarification selects controlled
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
