Adds the three remaining adapters plus the poller. Nothing is wired yet --
the composition root is the next commit -- so this is additive and the
legacy `app/scheduler/service.py` still serves production.
`run_launcher.py` is the pivot of the whole slice. The Gateway signals a
busy thread two ways -- `ConflictError` from the run manager, or an
`HTTPException(409)` from the route-level path -- which is why the legacy
scheduler service imported fastapi to tell them apart. Both are one domain
fact, and saying so here is what lets that import disappear without the
busy/failed distinction disappearing with it. Everything else becomes
`LaunchFailedError`, because the port promises the domain that nothing but
its two errors escapes. `CancelledError` is deliberately not caught:
shutdown is control flow, not a launch outcome.
`thread_lookup.py` narrows `ThreadMetaStore` to the one question this
context asks. `require_existing=True` is load-bearing -- the store's
default treats an absent row as accessible, which is right for a thread
not yet written and wrong for binding a task to it.
Both inherit their port explicitly, matching every other adapter in the
codebase including feedback's own anti-corruption layer, and both carry
the TODO naming the published contract that would replace them once the
upstream context has been through a slice of its own.
`run_outcome_mapping.py` implements no port: it is the inbound translation
the composition root will install on the completion hook, and it owns the
filtering the legacy hook did inline. Returning None means "this run is
none of the schedule context's business", so the service is simply never
called and needs no guard clauses.
`poller.py` keeps the two behaviours the legacy loop got right: a failing
poll must not end the loop (one transient "database is locked" used to
stop scheduling for the rest of the process life), and reconciliation must
not block startup.
One deliberate behaviour change: the legacy `start()` swept stale runs and
stuck once-tasks under separate try/excepts, so the first failing did not
stop the second. `reconcile_on_startup` is one call that lets failures
propagate -- the domain's position is that fatality is the caller's policy
-- so the poller's single except means a failed first sweep now skips the
second. Both end up logged and non-fatal, as before.
Tests: 50 new cases across the four modules, each port method called and
asserted on its return value. That is not decoration: inheriting a
Protocol means a misspelled method silently inherits its `...` body and
returns None, so the suite was verified by mutation -- renaming `launch`
and `exists_for_user` turns 16 and 6 cases red respectively.
Adopts the layout feedback landed in cb49dd67: secondary adapters live
under `app/adapters/<context>/`, one file per port, the file named after
the port in snake_case with the technology carried by the class name.
`app/infra/` is now gone entirely.
The rename also separates two meanings of "run" that shared one filename
space: `run_sql.py` held `ScheduledRun` (an execution record), while the
`run_launcher.py` still to come deals in Gateway runs.
task_sql.py -> scheduled_task_repository.py
run_sql.py -> scheduled_run_repository.py
spec_mapping.py -> spec_mapping.py (implements no port, so no rename)
Both SQL adapters now carry the `Secondary adapter (owned persistence)`
docstring marker -- this context owns both tables and writes its own
queries. `spec_mapping` says instead that it is a boundary mapping and
names its two callers. The package `__init__.py` is empty, so imports go
through the full path and a class's home file stays unambiguous.
Pure move: every top-level symbol was compared against its pre-move
original by AST dump (docstrings excluded), plus a separate per-class
method-name comparison, since a whole-class dump reports a docstring edit
and a renamed method the same way.
Also repoints three docstrings and one diagram that still named the
deleted `app/infra/` path.
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.
HEXAGONAL_ARCHITECTURE_zh.md now carries rules only: the two orthogonal
boundaries, the AWS three-folder mapping, the two kinds of secondary
adapter, and how the boundaries are mechanically enforced. The feedback
walkthrough, its known gaps, and its todo list move to a module document,
so the guide stays readable as more modules are migrated.
Two corrections to the guide, both of which would have misled a reader:
- Ports belong inside `domain/`, not beside it. AWS places `ports/` as a
subdirectory of `domain/` and describes the domain folder as "domain and
interfaces"; lifting ports into a third top-level layer would make the
domain depend on an outside package to declare its own needs. The guide
now states this explicitly, since the opposite reading is common.
- The walkthrough had the service calling RunLookup before building the
aggregate. The code does the reverse, and the order matters: validation
runs before any port call, so an invalid rating on a nonexistent run is
reported as InvalidRatingError rather than RunNotFoundError. The same
section also called that check authorization; it is referential
integrity, and authorization is the router's owner_check plus this check
taken together -- which is why the port takes no user_id.
FEEDBACK_DESIGN_zh.md is new and follows the SCHEDULE_DESIGN_zh.md shape:
the aggregate and its invariants, both ports and the conventions that
matter more than their signatures, the four use cases, both adapters, the
walkthrough, the test layering, an extension guide, and a pitfall list.
Three things it records that were not written down anywhere:
- The aggregate reads the system clock in its default factory, which
schedule deliberately avoids. Acceptable while the timestamp is only a
bookkeeping stamp and feeds no rule; noted with the condition that would
force a change.
- A repository that explicitly inherits its Protocol turns a misspelled
method into a silent None, because the inherited body is `...`. Hit for
real during the move. isinstance() cannot detect it, so asserting "the
port is satisfied" is not a substitute for asserting return values.
- RunLookup has no contract test against a real RunStore. A renamed key in
the dict RunStore.get() returns would turn every rating into a 404 with
the suite still green.
README.md indexes both under Quick Links, next to ARCHITECTURE.md.
Group them by bounded context instead of by technology, one file per
port, and align the directory name with the AWS Prescriptive Guidance
layout (entrypoints / domain-with-ports / adapters).
app/infra/persistence/feedback.py
-> app/adapters/feedback/feedback_repository.py owned persistence
-> app/adapters/feedback/run_lookup.py anti-corruption layer
`persistence/` promised a technology-first classification that its own
contents contradicted: RunStoreRunLookup lived there while its docstring
said "no new SQL". Splitting per port makes that distinction structural.
SqlFeedbackRepository and _tz_aware move unchanged -- verified by
comparing their AST against the original rather than by eye. run_lookup.py
additionally gains a RunStore annotation behind TYPE_CHECKING (the module
is imported lazily by the composition root, so this keeps the runtime
import cost at zero), a docstring stating that this context owns no table
and writes no SQL against it, and a TODO recording the condition under
which the body is replaced: when the run context publishes a contract of
its own, the RunLookup port itself does not move.
Each module docstring opens with a fixed marker so the two kinds of
secondary adapter stay greppable:
grep -rl "anti-corruption layer" app/adapters/
Filenames deliberately carry no sql_ / acl_ prefix: a prefix encodes an
implementation property, so switching storage would force a rename even
though the port -- and therefore the import path -- has not changed. The
class name already carries it. A prefix earns its place once one port has
several production implementations, which is not yet the case here.
app/infra/ held nothing else and is removed.
The inner ring is organised per bounded context; the outer ring was not,
so this context's adapters were split across app/infra/persistence/ and
app/infra/schedule/ with no principle separating them — one held the SQL
repositories, the other held the mapping they both consume, purely
because of what technology each touched.
Everything the outer ring provides to the schedule context now lives in
app/infra/schedule/, mirroring domain/schedule/. That also gives the two
remaining adapters an obvious home: the run launcher and the thread
lookup are neither persistence nor mapping, and would have needed a
third rule under the old layout.
Files are moved with git mv so history follows them. app/infra/
persistence/feedback.py stays where it is: it belongs to a separate
migration and moving it here would put that work in this diff. The
package docstring records the asymmetry rather than leaving it to be
rediscovered.
Two secondary adapters plus the spec mapping they share, and the first
real payoff of the port boundary: test_schedule_fakes.py becomes a
contract suite that runs all 31 cases against both the in-memory doubles
and the SQL adapters on a real sqlite file. A rule stated in a port
docstring now has to hold for both, and a divergence is a failure rather
than a surprise in production.
The queries come over unchanged. The claim statement's FOR UPDATE SKIP
LOCKED and both protect_terminal conditional writes are this module's
concurrency contract, not style, and the IntegrityError translation in
add() is what lets the service collapse a lost active-slot race into the
same outcome as its own non-atomic fast path.
Two things the adapters own that the domain deliberately does not. The
claiming process's identity is generated here, because who claimed a
task is an identity rather than a rule and nothing reads it back. And
`Unsupported schedule_type` is raised by the mapping, because
ScheduleSpec only accepts the enum -- structural checks belong to the
boundary, value rules to __post_init__, and both surface as the same
domain error so the router maps one family.
_to_domain introduces a failure mode the legacy repository did not have:
a row whose stored schedule no longer parses. Single-row reads let it
propagate; list reads skip and log, so one corrupt row cannot 500 an
entire listing.
The contract suite reaches past the port in exactly one place -- seeding
a task that already carries a claim, a shape claim_due would never
produce -- and says so where it does.
One conflict, in frontend/src/core/threads/types.ts: main's #4513
(preserve message order during long runs) made RunMessage.seq required,
while this branch had added the optional feedback field next to it. Both
changes are independent and both kept — seq is now required per main,
feedback stays optional.
No migration renumber this round; main added no new revision, so
0010_feedback_tags still chains cleanly after 0009_webhook_dedupe.
Verified: frontend typecheck + lint clean and 844 tests pass (the
required seq propagates through the test helpers main updated), plus
74 backend feedback/thread-message tests.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The design doc stopped at the model layer while ports.py and service.py
had already landed, so it described the inner ring as one third of what
is actually there and still called the other two "to be built".
Two new chapters. Ports covers the three contracts that matter more than
the signatures — another user's task reads as absent rather than
forbidden, only ThreadBusyError or LaunchFailedError may escape a
launch, and RunOutcome keeps the run runtime out of the domain — plus
the two deliberate absences (no Clock, no claimer identity) and the line
between single-threaded semantics, which the contract owns, and
atomicity, which it does not. The service chapter walks dispatch_task's
four exits as a diagram, explains why the global budget is not a
per-poll batch size, and records why the context change travels packaged
rather than behind a sentinel.
The rest follows: the migration map now shows the inner ring complete
and only adapters outstanding, the overview gains the discipline each of
the three layers is held to, the dev guide gains "add a use case" and a
warning against writing rules in the service, and the pitfalls and
glossary pick up what the new layers introduce.
* fix(frontend): preserve message order during long runs
* test(frontend): fix history pagination regression mock
* fix(frontend): validate thread history sequences
A review pass over domain/schedule found three genuinely untested
branches, all of which now have cases:
- ensure_launchable with a naive `now`. next_after already had this
covered; the delay floor did not, so a caller handing over a naive
clock reading could have had it shifted by the local offset unnoticed.
- update_task changing the prompt. Only the title path was exercised.
- _save finding the row gone. get_task saw it and save no longer does,
which is a concurrent delete; the caller must get the same not-found it
would have got a moment earlier rather than a None leaking out.
That takes service.py and every model module to 100%. The one remaining
uncovered line is croniter's naive-return guard, carried over verbatim
from schedules.py and unreachable with an aware input -- it now says so
instead of looking like an untested branch.
Also fills in the documentation the migration skipped: TaskStatus,
ContextMode and RunStatus arrived from the original draft without
docstrings while their newer siblings had them, and ScheduleService plus
four of its use cases were undocumented. Each now records the reasoning a
reader would otherwise have to reconstruct -- why RUNNING is not "the
agent is executing", why SKIPPED never passes through QUEUED, why
INTERRUPTED is not FAILED.
CRON_FIELD_COUNT stops being exported: it has no consumer outside the
module that defines it.
Four changes, all from the same observation -- the service was expressing
domain concepts in language the domain does not use.
DispatchOutcome replaces the Literal. The caller branches on all four
values and the SKIPPED/CONFLICT distinction is itself a business rule, so
it is domain vocabulary like every other enum in this context; a bare
string was the odd one out.
lease_owner leaves the domain entirely. It was a pure pass-through: the
service held it only to hand it to claim_due, which never reads it back.
Which process claimed a task is an identity, not a rule -- an adapter can
record one for diagnostics without the domain carrying it. lease_seconds
stays, because how long a claim survives genuinely changes recovery
behaviour.
The _UNSET sentinel is gone. It existed for one field: thread_id, the only
update parameter with a meaningful None. But thread_id and context_mode
always move together, so packaging them as ContextChange removes the
ambiguity and lets every other field use plain None for "not supplied" --
which also matches what the HTTP layer already does with exclude_none.
The previous `title: str = _UNSET` annotation was simply untrue.
pause/resume no longer route through a _transition helper taking an
unbound method. Two direct bodies plus a _save that raises on a missing
row read better, and _save documents why this stays a read-modify-write:
pushing "not while running" into a storage predicate would put the rule
beyond a zero-IO test and give it a second home. Closing that window
properly needs optimistic locking, which needs a schema change.
Also cleans three lint findings surfaced by a broader rule set than the
project enables: a regex metacharacter in a pytest match=, an unused
override parameter, and two deliberately-naive datetimes that now say so
with a noqa rather than looking accidental.
The input port of the context: every scheduled-task use case, orchestrated
over the four output ports. It holds no business rules -- each decision is
delegated to the aggregate -- and `test_schedule_service.py` runs the
complete lifecycle with no HTTP, no database and no run runtime, which is
the acceptance criterion this migration was for.
`dispatch_task` mirrors the pre-migration structure line for line,
including its comments, and makes exactly three substitutions: bare dicts
become domain objects, the HTTPException-409 sniffing becomes `except
ThreadBusyError`, and the status-derivation static methods become
aggregate methods. Its four exits and their two conflict paths are
unchanged, and the tests pin the collapse: the fast-path rejection and
the active-slot rejection must produce identical results, asserted over
every field of DispatchResult.
One behavioural narrowing, deliberate. The old code wrapped the launch
*and* its follow-up writes in `except Exception`, so a failing bookkeeping
write was recorded as a failed launch -- marking an execution that had
actually started as failed. The port contract admits exactly two escapes
from `launch`, so only the launch is guarded now and a genuine write fault
propagates instead of being misreported.
SchedulePolicy gains max_concurrent_runs and lease_seconds. Both are
operator-tunable thresholds the domain needs but must not read, which is
what that value object is for; the claiming process's identity stays a
constructor argument since it is an identity, not a threshold.
Four Protocols the domain declares and the outer ring will implement,
plus the two DTOs that keep infrastructure types out of the inner ring:
- ScheduledTaskRepository / ScheduledRunRepository, exchanging domain
objects rather than the bare dicts the current repositories return
- RunLauncher, whose contract is that only ThreadBusyError or
LaunchFailedError may escape -- that translation is what keeps the run
runtime and the web framework out of the domain
- ThreadLookup, one method rather than the whole thread store
- LaunchedRun and RunOutcome, so the completion path stops taking a
runtime record the purity test would reject
Two deliberate departures from the earlier sketch. There is no Clock
port: `now` is already an explicit parameter throughout, so the domain
never reads a clock and the tests are already deterministic -- adding
one would only create a second source of truth for the same value. And
`record_launch` is not expressed as `save(task)`, because
`protect_terminal` makes it a compare-and-set against a concurrently
finalizing run; a read-modify-write through the aggregate would
reintroduce the race the flag exists to close.
The in-memory doubles model the active-slot rule rather than skipping
it: a double that never refuses a second active run would let the
service's conflict collapse go untested. Their semantics are pinned by
test_schedule_fakes.py, which becomes the contract suite once the SQL
adapters land and both tiers run the same cases.
Concurrency is out of scope for the doubles and says so in their module
docstring -- it stays covered against a real database in
test_scheduled_task_dispatch_race.py.
`_finalize_skip` carries a task's launch bookkeeping over unchanged by
reading the current values off the task dict and passing them back into
`update_after_launch`. But repository dicts hold ISO *strings* for
timestamps -- `_row_to_dict` runs every datetime column through
`coerce_iso` -- while `scheduled_tasks.last_run_at` is a DateTime
column, so that round trip fed a string into a datetime bind parameter
and raised StatementError.
It only reproduces once the task has launched at least once: before
that `last_run_at` is NULL, which the column accepts. Every existing
dispatch test seeds a fresh task, so none of them reached it.
The blast radius was the whole poll cycle rather than the one task: the
exception escapes `dispatch_task` into `_run_loop`, so every task still
queued behind it in that round goes undispatched, and the failing task
holds its lease in `running` until it expires. The skip tombstone was
already written by then, so the run history was left inconsistent with
the task row.
Coerced at the call site instead of loosening the repository's parameter
type: the skip path is the only one that round-trips a stored timestamp,
every other write passes `now` straight through.
First step of the scheduled-task context's hexagonal migration: the
inner-ring model, with zero infrastructure dependencies.
- ScheduleSpec / SchedulePolicy value objects
- ScheduledTask aggregate root
- ScheduledRun aggregate
- 9 domain errors, 5 enums
Rules are migrated verbatim from their current homes, each method's
docstring citing the source line: timezone/cron/next-run calculation
from deerflow/scheduler/schedules.py, context-mode and re-arm rules from
routers/scheduled_tasks.py, and the four status-derivation rules from
app/scheduler/service.py.
Two things previously held by convention are now enforced by
construction. Validation and normalization live in __post_init__, so
building a ScheduleSpec field-by-field cannot bypass them. The skipped
tombstone is a separate factory, so it can never be written as the
transient queued row that would collide with uq_scheduled_task_run_active.
The domain does not serialize itself: mapping the stored schedule_spec
JSON in and out stays with the adapter layer, keeping Mapping[str, Any]
out of every domain signature.
Production code still runs through app/scheduler/service.py -- this
commit adds no call sites and changes no behavior.
`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>
The commented Crawl4AI web_fetch example still describes the pre-0.9 server:
it says JWT auth is off by default, marks `token:` as only needed "if the
server has JWT auth enabled", and pins 0.8.6 in the docker run line.
Crawl4AI 0.9.0 made the Docker API server secure-by-default. Auth is on for
every request except GET /health, and a server started without
CRAWL4AI_API_TOKEN binds 127.0.0.1 only. Following the current example against
a current image therefore yields either HTTP 401 on every fetch, or a server a
containerised DeerFlow cannot reach -- with no hint that auth is the cause.
0.8.6 is also worth moving off: 0.8.7 fixed two pre-auth RCEs (CVSS 9.8), and
0.8.8/0.8.9 closed SSRF gaps in the same server.
No code change is needed -- Crawl4AiClient already sends
`Authorization: Bearer <token>` whenever `token` is set, so this is purely
the example catching up with the upstream server. Comment-only, so
config_version is unchanged.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Rebase the feedback migration onto main's new chain tip again: main added
0009_webhook_dedupe (also chained after 0008_thread_operation_kind), so
0009_feedback_tags becomes 0010_feedback_tags with
down_revision=0009_webhook_dedupe, keeping the chain linear.
Conflicts resolved:
- Five head-pin tests take main's version with the pin bumped to
0010_feedback_tags.
- test_thread_messages_page.py keeps this branch's feedback_service
wiring over main's feedback_repo wiring, but stubs both service
methods so the thread-grouped path main added coverage for stays
stubbed (latest_per_run_in_thread alongside latest_for_runs).
- test_thread_messages_feedback.py keeps both sides' imports; each is
used (Feedback for the fixture, EditReplayVisibility for the run
manager stub).
Verified: 79 tests across the conflicted files plus the migration and
bootstrap suites, and 54 feedback tests, all pass.
Co-Authored-By: Claude Opus 4.8 <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>
* feat(checkpoint): production-shaped full/delta benchmark with configurable snapshot frequency
- Group benchmark scripts into per-family folders (checkpoint/, sandbox/)
- Extract shared benchmark infrastructure into checkpoint_bench_common.py
- Add checkpoint_delta_snapshot_frequency config (default 1000, process-frozen);
freeze it in make_lead_agent and DeerFlowClient; key the state-schema
adaptation cache by resolved frequency
- New bench_production.py: per-case child processes run N ainvoke turns through
the real lead-agent graph (scripted deterministic model, real AsyncSqliteSaver),
then measure GET /state + POST /history through the real Gateway route stack
in one event loop (httpx ASGITransport), cold/warm accessor-cache split,
cross-mode digest gates
- New summarize_production.py: delta/full ratios plus decision metrics
(snapshot_write_spike, cache_effect_ms, checkpoint_write_share,
auto-discovered history per-limit ratios)
* fix(checkpoint): address production benchmark review
Four findings from @willem-bd on #4401:
- FeedbackDialog stays mounted across messages, so selected tags and the
comment survived an ESC/click-outside dismiss and pre-filled the next
thumbs-down. Reset on every close path via a wrapped onOpenChange.
- Neither the dialog's handleSubmit nor handleDialogSubmit caught a failed
enrichment PUT, so a rejection went unhandled and the user got no signal.
Catch in the dialog (where the rejection lands), toast, and keep the input
for a retry.
- rate_run awaited the RunLookup port before Feedback.create validated the
rating, contradicting its own "before any I/O happens" docstring: an
invalid rating on an unknown run surfaced as RunNotFoundError. Validate
first, restoring the legacy router's 400-before-404 ordering. The service
test now uses an unknown run id so it actually pins that order.
- InMemoryFeedbackRepository moved out of test_feedback.py into
tests/feedback_fakes.py (with FakeRunLookup) so two test modules share it
without one importing the other; conftest states the tests-dir sys.path
dependency explicitly, which also makes it work under
--import-mode=importlib.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Rebase the feedback migration onto main's new chain tip: main added
0008_thread_operation_kind (also chained after 0007), so
0008_feedback_tags becomes 0009_feedback_tags with
down_revision=0008_thread_operation_kind, keeping the chain linear.
The five head-pin test conflicts resolve to 0009_feedback_tags on top
of main's versions (preserving the new operation_kind assertions).
Verified: 42 migration/bootstrap tests, 54 feedback tests, and the
full frontend suite (797) pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Merging main rebased this branch's feedback migration to
0008_feedback_tags on top of main's 0007_scheduled_run_active_index,
so the chain head moved past the pin in main's new dedupe test. The
test runs `upgrade head` and asserts the chain tip; its actual subject
(the dedupe pass + partial unique index) is unaffected.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(runtime): linearize delta-mode checkpoint resume
Resuming a run from an older checkpoint forks the lineage, and in delta
mode that fork's state cannot be materialized correctly: the delta
history walk collects every pending_writes entry stored on each on-path
ancestor, but a shared parent also carries the writes of the sibling
child that was abandoned. Those writes replay into the fork, so the run
starts from a message list that still contains the answer it was meant to
replace — regenerating in a branched thread surfaced this as the
superseded assistant message reappearing beside the new one after a
reload. All three saver implementations are affected, so write-to-child
ownership is a gap in the upstream delta contract rather than one
saver's slip.
Rather than reimplement that walk, express the fork as what it means:
materialize the requested checkpoint's state, write it as an Overwrite on
the current head (which has no siblings), and run linearly. The abandoned
turn stays in history as the rewritten head's ancestry.
This runs after the rollback point is captured, so cancel-with-rollback
still restores the real pre-run head, and fails closed — an unreadable
resume checkpoint raises instead of falling back to the corrupt fork.
Full mode keeps forking: its checkpoints carry complete channel_values
and need no replay.
* fix(runtime): restore complete delta resume state
* fix(runtime): linearize delta rollback restoration
* Apply suggestions from code review
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* fix(runtime): serialize delta resume preparation
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Opening the dev stack on a LAN address or a proxied hostname serves the
SSR HTML but never hydrates: Next.js answers /_next/*, /__nextjs_font/*,
and HMR with 403 for any host it was not started on. The page renders, so
it looks up — but no client handler is attached, and the login form's
onSubmit never fires. It reads as "login is broken" rather than as an
asset problem, and the only clue is a warning in the dev-server log.
Wire Next's allowedDevOrigins to a new DEER_FLOW_DEV_ALLOWED_ORIGINS env
var. Unset by default, so the localhost-only default is unchanged; it is
also dev-only, as Next ignores allowedDevOrigins in production builds.
Entries are reduced to the bare host that allowedDevOrigins matches
against, since an entry pasted from the address bar as
"http://192.168.1.10:2026" would otherwise match nothing and leave the
operator with the same 403 they were trying to fix.
Reported in #54 and #203.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(frontend): restore resizing for the artifacts and sidecar panels
#3934 replaced the right panel's ResizablePanelGroup with a fixed CSS grid to
animate open/close, which removed the drag handle; #4187 then reintroduced a
resizable group for the browser panel only. The artifacts and sidecar panels
have had no way to resize since, while the browser divider still drags.
All three right panels now share one panel group, so there is no per-panel-kind
layout fork. Open/close goes through the side panel's collapse()/resize() so the
width still animates, and a dragged width survives closing and reopening.
Three library-specific constraints, each found by a failing test:
- the size transition is applied from the group as
[&>[data-panel]]:transition-[flex-grow], because <ResizablePanel className>
lands on an inner wrapper while the element the library sizes is its own
[data-panel] div;
- reopening uses resize(remembered) rather than expand(), which falls back to
minSize until the library has recorded a size, and the remembered width is
read before collapse() because the closing animation reports shrinking sizes;
- during the animation the content is held at its final width in cqw and
clipped, as the previous grid layout did — letting it reflow every frame makes
the message list re-run its scroll-to-bottom and re-wraps the sidecar
composer.
Fixes#4465
* fix(frontend): remove unreachable panel max size
