mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-09-29 16:06:21 +00:00
15 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Wenchao An
|
e2f19d8335
|
feat(plugins): full-stack plugin APIs and bookmarks (#5647)
* feat(plugins): add full-stack contributions and bookmarks example * ci(plugins): provision bookmark gateway for browser tests * fix(plugins): authenticate module downloads through configured backend * fix(plugins): isolate contributions and localize extension UI * fix(plugins): preserve bookmark agent routing and contain async callbacks * fix(plugins): pin durable batch workers to app extension snapshots |
||
|
Totoro
|
cd0e74edaf
|
fix(scheduler): reconcile stuck once tasks from committed run outcome (#5035)
* fix(scheduler): reconcile stuck once tasks from committed run outcome Restart recovery (cancel_stuck_once_tasks and the multi-instance reconcile_stuck_once_tasks) blindly flipped every stuck once-task to 'cancelled'. When handle_run_completion crashed between its two transactions, a once-task whose run had already committed 'success' was permanently reported as cancelled. Both reconciliation paths now read the latest scheduled_task_runs row without a status filter and finalize the parent to match: success -> completed (last_error cleared), failed -> failed with the run's error, interrupted -> cancelled with the run's error when present, skipped -> cancelled (no work performed). Active occurrences (queued/launching/running) are left untouched — a concurrent completion or a later recovery pass will finalize them once the run reaches a terminal state. Tasks without a terminal run row keep the previous generic cancellation. Review follow-ups (willem-bd / Huixin615): - Extract _finalise_once_task_from_run() so both recovery paths share one outcome mapping (no more drift between single- and multi-instance paths). Returns bool (True = finalised, False = active/no-op) for explicit counter management at call sites. - Fix a no-op (`run_row.error or None` -> `run_row.error`) in the skipped branch. - Drop the unused `status` parameter from the test task helpers. - Use TERMINAL_RUN_STATUSES / ACTIVE_RUN_STATUSES constants (local copies to avoid circular import; kept in sync with scheduled_task_runs.sql). - [P1] Read the latest run AFTER acquiring the parent task row lock, not from a pre-lock batch snapshot. The latest-run lookup now runs per task under the lock with populate_existing so a concurrently committed status is read back fresh. - [P2] Race tests now use monkeypatch to actually enter the race window: _intercepted_fetch commits success in a separate session at the moment the per-task fetch fires, so a reverted pre-lock batch implementation fails the test, while the current post-lock implementation passes. - [P1] Do not finalize parent for active occurrences. A non-terminal scheduled occurrence means the run is still in progress — the parent must be left untouched until the completion path or a later recovery pass establishes a terminal outcome. - [P2] Add cancel_stuck_once_tasks to the single-instance poll loop so stuck once-tasks are not left permanently "running" when the startup sweep fails (mirrors multi-instance _reconcile_active_state behavior). - Fix stale docstrings in cancel_stuck_once_tasks and _fetch_latest_run. Adds regression tests for multiple historical runs (older success + newer skipped/active) on both paths, monkeypatch-based race tests that prove a concurrent completion committing success is reflected as completed, and active-run tests that verify the parent is left unchanged. Documents the behavior in AGENTS.md. Fixes #5034 * fix(scheduler): address review comments on completion-consistency fix - _fetch_latest_run: drop arbitrary id DESC tie-break; order by scheduled_for DESC (deterministic recency on schedule position) - _finalise_once_task_from_run: annotate bool return type - Centralize TERMINAL/ACTIVE_RUN_STATUSES in scheduled_tasks/model.py; stop duplicating them in scheduled_tasks/sql.py and scheduled_task_runs/sql.py (removes stale circular-import workaround) - cancel_stuck_once_tasks: run unconditionally in single-instance poll loop (remove try/except swallow) - tests: pin created_at/scheduled_for in _create_run so recency ordering is actually exercised; correct docstrings that described the active-occurrence branch as 'generic cancel' instead of 'left unchanged' * fix(scheduler): correct finalizer return annotation * fix: order scheduled task runs by creation time * fix(scheduler): stabilize latest run reconciliation ordering * fix(scheduler): order latest runs by creation time * test: update trace scheduler stub * fix(scheduler): clarify reconciliation diagnostics Signed-off-by: Totoro-qaq <279883115+Totoro-qaq@users.noreply.github.com> * fix(scheduler): fail closed on startup recovery Keep single-instance parent reconciliation at startup so it cannot race manual admission. Propagate recovery failures through the Gateway lifespan before channel startup, preventing a half-started scheduler. Tests cover both recovery failure stages and a queued occurrence that survives startup before the ordinary poll drain launches it. Signed-off-by: Totoro-qaq <279883115+Totoro-qaq@users.noreply.github.com> * fix(scheduler): order occurrences and fence stale parent writes Allocate per-task occurrence sequences under the parent lock and guard parent projection across launch, recovery, completion, and queue failure paths. Track launch accounting separately so stale occurrences are counted once without replacing newer results. Commit completion and accounting atomically, preserve legacy history, and cover migrations and reordered execution on SQLite and PostgreSQL. * fix(scheduler): tighten completion projection and launch fencing diagnostics Share the once-task outcome mapping between completion and both recovery paths, validate the terminal status before opening the completion transaction, leave cron parent status untouched on completion, log the fenced launch update when an occurrence does not belong to the launched run, and drop the README capability line. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(scheduler): compare caller time only against unsequenced occurrences Among sequenced rows the parent-locked occurrence_seq is the only recency key. An unsequenced row can only be legacy history or an admission by a pre-upgrade Gateway writer, so recovery prefers it over the sequence winner only when its caller timestamp is later, which is the previous ordering for that pair. A rolling upgrade therefore degrades to the pre-sequence behaviour instead of ranking every pre-upgrade admission below every sequenced one. Document that boundary instead of requiring every Gateway writer to stop before the upgrade. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(scheduler): gate once-task recovery on the same projection rule Recovery now finalises a once-task parent only from the occurrence that can_project() accepts: the highest sequenced occurrence whenever one exists, or the timestamp-latest row for a task whose history is entirely unsequenced. An unsequenced row admitted by a pre-upgrade writer can no longer cancel a parent whose sequenced occurrence is still live, nor stall finalisation of a parent whose sequenced occurrence already completed. Document that pre-upgrade instances project their own admissions during a rolling upgrade. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(scheduler): defer once-task recovery while any occurrence is live uq_scheduled_task_run_active allows one non-terminal occurrence per task, so a live row is the task's newest admission whatever its caller clock and whether it carries a sequence. Both once-task recovery paths now probe for any active occurrence after the fresh latest-run read and leave the parent untouched while one exists; cancel_stuck_once_tasks also locks the parent row so admission cannot insert a queued occurrence between that probe and the commit. Once no occurrence is live, the sequence winner decides and a terminalised unsequenced row never overrides it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(persistence): follow the local head past canonical 0019 Main's forward-revision tests assumed 0019_thread_incarnations was the local chain head. With 0022_scheduled_occurrence_seq chained after it, seed the canonical-0019 shape explicitly, assert the real head where a database is upgraded, derive the 0020 rollback binary's revision set from the ancestors of its head, and step the PostgreSQL restart scenario back to canonical 0019 before the rollback binary restarts. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * docs(migrations): describe the chain through 0022_scheduled_occurrence_seq The rolling-forward section still ended the local chain at canonical 0019; it now names 0022_scheduled_occurrence_seq as the head and lists it among the revisions the 0020 rollback-floor binary does not know. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(scheduler): accept CI's sync Postgres URL in occurrence fixtures CI hands over TEST_POSTGRES_URI as postgresql://...?sslmode=disable. The occurrence, ordering and 0022 migration fixtures built async engines from it directly, so SQLAlchemy chose psycopg2, which is not installed. Normalize the scheme to postgresql+asyncpg and drop libpq-only query keys, matching the existing 0019 migration tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(scheduler): keep the backend AGENTS.md chain within its budget The middlewares guidance chain was already above the hard limit on main, so any added byte in backend/AGENTS.md fails the agent guidance check. Leave backend/AGENTS.md identical to main and record the recovery projection rule in the 0022 migration entry, which already describes the occurrence fields. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Signed-off-by: Totoro-qaq <279883115+Totoro-qaq@users.noreply.github.com> Co-authored-by: Totoro-qaq <279883115+Totoro-qaq@users.noreply.github.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> |
||
|
Aari
|
47b258ebd7
|
feat(mcp): add ordinary durable task driver (#4690)
* feat(mcp): add durable task runtime foundation * fix(chart): sync embedded config version * fix(mcp): isolate task polls during shutdown * feat(mcp): track consecutive poll errors on mcp_tasks poll_attempt_count grows on every claim (successful polls included), so it cannot drive a failure backoff without misjudging normal long tasks. Add consecutive_poll_error_count: incremented when a claim is released after a poll error, reset to zero by any applied snapshot. The backoff/terminal policy that consumes it lands with the first concrete driver. * fix(mcp): harden durable task lifecycle * feat(mcp): add ordinary durable task driver * test(mcp): address durable task review feedback * fix(mcp): preserve submit tool descriptions * fix(mcp): bound remote task calls * fix(mcp): bound persisted task payloads * fix(mcp): preserve task tool error details * fix(mcp): enforce durable task boundaries * test(mcp): cover task config snapshot lifecycle --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Nan Gao
|
7389331e65
|
feat(extensions): observe task lifecycle and system model calls (#4684)
* feat(extensions): observe task lifecycle and system model calls PR 1 (#4636) gave extensions a middleware chain, and a middleware only sees what passes through the agent graph. Two runtime surfaces stay invisible to it: when a lead run or a subagent begins and ends, and the DeerFlow-owned model calls made outside the graph. This slice adds both, with no new Gateway surface -- routers, services, and the reference extension stay in PR 3. Contract (deerflow-extension-api 0.1.1) --------------------------------------- Two contribution kinds join `middlewares` on the registry: `task_lifecycle` (`on_task_start` / `on_task_stop`, receiving a `TaskInfo` and a conservative `TaskOutcome` of completed / aborted / failed) and `system_model_observer` (`on_system_model_call`, receiving a `SystemOperationKind`, a `SystemModelRequest` snapshot, and a `SystemModelResult` carrying either the response or the provider exception plus a duration). `SystemModelRequest.messages` normalizes to a tuple at construction. Goal evaluation and memory extraction pass a message list while title generation and summarization pass one prompt string, and a bare `str` already satisfies `Sequence` -- without normalization an observer iterating `request.messages` would silently walk characters. Copying also makes the frozen snapshot immutable in fact rather than only by declaration, since observations may run after the call site returns and keeps mutating its own list. Registry marks and rollbacks become per-bucket and positional, so an `install()` that fails after registering two different kinds cannot leave one of them behind. `needs_task_store` now covers all three kinds: a deployment that registers only lifecycle hooks still gets a task store. Task lifecycle -------------- The lead worker notifies start after the run has started and stop after completion persistence and the completion hook, but before clearing the finalizing barrier and publishing the stream end -- holding the barrier across stop is what keeps a same-thread replacement run from overlapping this task's lifecycle. Cancellation raised out of the stop notification is deferred, not propagated in place, so a cancelled run still clears the barrier and emits its end frame. A subagent with a parent `run_id` wraps its execution in the same pair inside `finally`, reporting `parent_task_id` so a delegation tree is reconstructable; a subagent without a `run_id` (embedded client, standalone LangGraph Server) logs and skips rather than inventing a parent. Contributors run in registration order inside one shared 3s budget and every failure is logged and failed open. System model calls ------------------ Four kinds cover the model calls the middleware chain cannot see: goal evaluation, memory extraction, title generation, and summarization. Each site reports both terminal paths without changing the provider exception the host observes, short-circuits on `has_system_model_observers`, and passes the live task store when the runtime has one (detached work gets an isolated store). The sync summarization half stays unobserved on purpose -- it and its only host caller are the sync side of an async-only runtime, so notifying there would block a thread on a call site the host never reaches; the reason is recorded at the call site. The DeerMem backend must stay vendorable and cannot import the extension API, so it reports through a new `MemoryCallbacks.on_memory_llm_result` host hook that the DeerFlow-side callbacks translate into an observation. Notification loop ----------------- Extension resources must be touched on the loop that created them, but subagents can execute on isolated loops and DeerMem runs on a worker thread. The Gateway registers its serving loop before any runtime dependency starts and resets it last through the exit stack, so every startup-failure and cancellation path is covered. Awaited hooks raised on another loop are dispatched across with `run_coroutine_threadsafe` and awaited under the same budget; synchronous sites submit fire-and-forget work. Shutdown stops accepting detached observations before the memory flush -- that flush runs on a worker thread and can emit memory observations -- while keeping the loop alive for awaited task hooks until run and subagent drain completes. Tests ----- `test_extension_task_lifecycle.py`, `test_extension_subagent_lifecycle.py`, and `test_extension_system_model_calls.py` cover ordering, fail-open, budget exhaustion, snapshot binding under a concurrent singleton replacement, the loop-dispatch and shutdown-suspension paths, and both terminal paths at every call site. `test_gateway_run_drain_shutdown.py` pins the stop-before-barrier and drain ordering. * fix(extensions): decide notification fail-open by origin, observe cancellation `_notify_each` only guarded `Exception`, so a contributor letting a `CancelledError` escape — an extension implementing an internal timeout with cancellation, say — skipped its successors and reached the worker's deferred-interrupt path, ending an otherwise successful run as cancelled. Fail-open is about where a failure came from, not its base class: only a genuine cancellation of the host task increments `Task.cancelling()`, so propagate on that and contain everything else. `KeyboardInterrupt` / `SystemExit` still propagate. `observe_system_model_call` skipped observers on cancellation for the same base-class reason, leaving goal / title / summarization silent on a terminal path that is routine — interrupt/rollback admission and shutdown both cancel the run task, with the provider tokens already spent. Awaiting observers there is unreliable (a repeated cancel interrupts that await before any of them runs), so report through the same non-blocking submission the synchronous memory bridge uses, then propagate the cancellation untouched. DeerMem keeps `BaseException` around its provider call, now with the reason recorded: that path runs on a worker thread, where cancelling the awaiting side never interrupts the running thread, so `CancelledError` cannot arrive at all. Its host-hook wrapper narrows to `Exception` — only the hook's own failures are non-fatal, and an observability path must not swallow a process teardown signal. * fix(extensions): warn on budget exhaustion, scope observer logs by task, propagate teardown Review response on #4684: - The memory observation bridge caught BaseException, which would swallow a teardown signal raised while dispatching; it now catches Exception, matching the boundary the DeerMem-side call site documents and tests. - A notification-budget timeout raised mid-hook fell into the generic hook-failure path and logged an asyncio-internal traceback; it now logs a warning like the pre-hook budget skip, while a TimeoutError a contributor raises on its own stays classified as a hook failure. - System model observer logs passed the operation kind as the task id, so log lines said "task goal/title/..."; they now carry the task scope id alongside the kind. |
||
|
Xinmin Zeng
|
f2e832330e
|
fix(sandbox): enforce disabled skills in filesystem views (#4178)
* fix(sandbox): project enabled skills into sandbox views * fix(skills): keep projection mutations consistent * fix(skills): fail closed on projection errors * fix(skills): isolate per-scope failures during boot projection rebuild rebuild_all_skill_projections() propagated any exception from the public rebuild or from a single user's rebuild straight out of the gateway lifespan startup, uncaught. A single broken user directory (bad permissions, corrupted _skill_states.json, unreadable content) would therefore abort gateway boot for every user, not just that one - _rebuild_*_locked already fails closed internally (clears the view and re-raises), so the boot loop only needed to stop treating that re-raise as fatal. Each scope's rebuild now fails closed independently and boot continues; a scope left empty by a boot failure self-heals on the next sandbox acquire via ensure_skill_projections(). Also patches deerflow.skills.projection.rebuild_all_skill_projections in the memory-flush lifespan test fixture, matching the two sibling fixtures in the same file — this call is now on the lifespan startup path and the fixture's minimal SimpleNamespace config predates it. * test(skills): update authz test for the projection-aware public toggle _persist_shared_skill_state (introduced earlier in this branch) reads the shared extensions_config.json fresh from disk under the projection lock instead of through the cached get_extensions_config() singleton - that's the whole point of the fix (stale worker caches must not clobber another worker's concurrent update). The name no longer exists on the skills router module, so the test's monkeypatch of it started raising AttributeError instead of exercising the endpoint. The mock storage in this test isn't a real LocalSkillStorage instance, so _persist_shared_skill_state's projection-mutation branch is already skipped (nullcontext) and it falls back to a fresh ExtensionsConfig() for the nonexistent tmp config_path - no replacement monkeypatch needed. * fix(sandbox): make skill projection ensure best-effort in acquire acquire() called _ensure_skills_projection() directly, outside any try/except, in both LocalSandboxProvider and AioSandboxProvider. Every other skill-mount setup path in these providers has always caught exceptions and logged a warning rather than failing sandbox acquire outright (e.g. when config.yaml can't be resolved) - these two new call sites broke that contract, so any projection failure (including simply not having a config.yaml, as in CI's test environment) now failed acquire() itself instead of just leaving skill mounts off. _ensure_skills_projection now catches its own exceptions and returns None; both providers' callers already tolerate that (a None projection skips the skill-specific mounts, matching the existing degrade path) after making _append_public_skill_mapping and the custom/legacy mount block in LocalSandboxProvider explicitly None-safe. Caught by running the full suite with config.yaml removed, matching CI's environment - not caught locally because a real config.yaml was present, masking the failure. * fix(sandbox): make E2B skill projection mounts best-effort _skill_projection_mounts called ensure_skill_projections with no guard, unlike Local/AIO's _ensure_skills_projection. A raise propagated out of _apply_mounts before the configured-mounts loop ran, so a skills projection failure dropped the operator's own configured mounts too - only caught by create()'s outer warning, with nothing applied at all. Swallow here and return an empty mount list on failure, matching the Local/AIO pattern: still fail-closed for skills, but no longer widens the blast radius to unrelated configured mounts. Review feedback from PR #4178. * docs(skills): document projection trade-offs flagged in review - _update_tree_digest: note the metadata-only (not content) hashing trade-off and why runtime writes through this codebase are still covered regardless (rebuild-under-lock + rename always changes inode). - LocalSandboxProvider.acquire: note the acquire-time self-heal cost (cheap on a fresh manifest, ~400ms rebuild under lock on stale/drift). - skill_projection_mutation: drop the no-op except-Exception-then-raise; a raise from the mutation already propagates past the yield with the view left cleared, no explicit re-raise needed. - provisioner README: spell out that hostPath skills volumes require the gateway and K8s node to share DEER_FLOW_HOST_BASE_DIR (single-node or shared storage), and that the custom/legacy volumes' hostPath type Directory (not DirectoryOrCreate) makes a violation of that assumption a visible Pod-creation failure instead of a silent empty mount. Review feedback from PR #4178. * fix(skills): lazily repair user projections * fix(skills): close projection review gaps * fix(skills): refresh user projection enable state * fix(skills): close projection review follow-ups * fix(skills): preserve state across projection writes --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
qin-chenghan
|
795af20a6b
|
feat(memory): built-in FTS5/BM25 retrieval adapter (#4360)
* 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> |
||
|
Daoyuan Li
|
d2b5f884e3
|
fix(channels): buffer GitHub follow-ups during busy runs (#4133)
Queue comments received while a run is active, then submit one deduplicated follow-up after it finishes. Failed drains are requeued and watcher tasks stop cleanly with the channel manager. |
||
|
lllyfff
|
01a89f2379
|
[feat] memory: pluggable MemoryManager interface for backend onboarding (#4326)
* refactor(memory): pluggable MemoryManager interface for backend onboarding
Optimize the MemoryManager interface layer so new backends (mem0/openviking)
onboard with less code and the contract stays stable as capabilities are
added. A minimal backend now implements only from_config + add + get_context
(verified by test_memory_manager_interface.py::_MinimalBackend onboarding via
the factory); the factory no longer knows a backend's private hooks.
- MemoryManager: ABC -> pydantic BaseModel; three-tier methods (tier-1
add/get_context abstract; tier-2 management defaults; tier-3 optional hooks
warm/reload/fact + on_pre_compress/on_turn_start). Dropped 3 self-serving
hooks. 6 hasattr probe sites -> direct call + try/except NotImplementedError.
- from_config classmethod: factory thins to resolve + inject storage_path +
collect host hooks + call from_config; DeerMem-specific hook consumption
moved from factory to DeerMem.from_config.
- Invariants: @model_validator (mode='tool' requires search via supports_search
ClassVar); DeerMemConfig storage_path-is-file check moved here from factory.
- Async: aadd/aget_context/asearch default to the sync path (speculative).
- Callbacks: MemoryCallbacks + LangfuseMemoryCallbacks; on_memory_llm_call
subsumes tracing_callback (same signature/timing/mutation); deleted the
tracing_callback field. DeerMem decoupled from langfuse (portability).
- noop keeps read-op empty overrides (avoids router 500s on the
disable-memory-via-noop path); only delete/export inherit the base raise.
Behavior preserved: 661 passed / 13 skipped. Docs: backends/README.md rewritten
(three-tier + from_config + callbacks); samples README updated; removed stale
private doc paths.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(memory): 501 on unsupported read/manage endpoints + accurate warm log
Review follow-up on the three-tier MemoryManager refactor.
- Read/manage endpoints (GET /memory, /memory/export, /memory/status,
DELETE /memory, POST /memory/import) and the /memory/reload fallback now
catch NotImplementedError -> 501, matching the fact-CRUD endpoints. The
hasattr->try/except migration had skipped these: they were @abstractmethod
before (every backend implemented them, so they never raised), so once they
became tier-2 default-raise a minimal backend (only add + get_context) hit a
raw 500 -- there is no global NotImplementedError handler. get_memory is
shared via _get_memory_or_501 (covers /memory, export, status, reload
fallback). noop is unchanged: its read-op empty overrides never raise.
- warm() base default returns None (tri-state: True=warmed, False=failed,
None=nothing to warm) so the Gateway lifespan logs "skipping" for a
non-DeerMem backend (e.g. noop) instead of the inaccurate "warmed
successfully" it never earned. DeerMem.warm keeps True/False.
- Tests: 6 router 501 tests (read/manage + reload fallback) + 2 lifespan
warm-log tests (None->skipping, False->warning); conformance/pluggable
assert warm() is None.
705 passed / 13 skipped; lint clean.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(memory): review follow-ups - search-flag consistency, client reload, backend_config purity
Address review feedback on the three-tier MemoryManager refactor:
- [Medium] supports_search/search drift: the invariant now requires the
supports_search ClassVar flag to MATCH whether search() is actually
overridden (type(self).search is not MemoryManager.search), so the flag
can't drift from the impl. Catches both directions at instantiation: a
backend that overrides search() but forgets supports_search=True (was a
misleading tool-mode rejection), and one that sets the flag without
overriding (was a runtime NotImplementedError on the first memory_search).
noop sets supports_search=True to match its search() override. Conformance
adds drift + consistent-backend tests.
- [Low] client.reload_memory fallback: wrap the get_memory fallback so a
minimal backend (only add + get_context) surfaces a clean NotImplementedError
("implements neither reload_memory nor get_memory") instead of an uncaught
propagation -- mirrors the router's 501. Test added.
- [Low] backend_config purity: DeerMem.from_config restores backend_config to
the pure data the host passed after model_post_init parses the injected hooks
into DeerMemConfig (self._config, PrivateAttr); the field stays serializable
(no callables/LLM) and matches the README ("host hooks NOT in backend_config").
Test asserts purity + hooks wired.
- [Low] CHANGELOG: breaking-change note that mode='tool' + non-search backend
now fails fast at startup (was silently empty) so operators recognize it on
upgrade.
- [Nit] .gitignore: drop the env-specific .tmp-pytest/ entry (--basetemp is
local-only, not make test/CI).
709 passed / 13 skipped; lint clean.
Co-Authored-By: Claude <noreply@anthropic.com>
* docs(changelog): correct memory tool-mode fail-fast note
The CHANGELOG entry said mode='tool' + a non-search backend "(e.g. noop)"
fails fast at startup, but noop overrides search() (returns []) and sets
supports_search=True (required by the consistency invariant), so noop IS
search-capable and noop+tool does NOT fail fast. The fail-fast only affects a
custom backend that onboards without overriding search(). Reworded to drop the
misleading noop example and state both shipping backends implement search().
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: Claude <noreply@anthropic.com>
|
||
|
Tu Naichao
|
959bf13406
|
fix(memory): flush memory queue on graceful shutdown to prevent loss (#4181)
* fix(memory): bounded shutdown flush via MemoryManager.shutdown_flush Re-applies the memory-queue shutdown drain on top of the pluggable MemoryManager abstraction (#4122): the old top-level MemoryUpdateQueue singleton is gone, so the drain is now a backend contract instead of host code reaching into the queue. - MemoryManager ABC: shutdown_flush(timeout) -> bool. Every backend implements a bounded graceful-shutdown drain. - DeerMem: queue.flush_sync (daemon-thread + Event.wait hard timeout for the uninterruptible sync LLM call; joins an in-flight worker first so contexts a debounce Timer already pulled out are not lost on exit; skips inter-item sleep on the drain path; per-item succeeded/failed count), exposed via shutdown_flush. - noop: shutdown_flush is a clean no-op success. - Gateway lifespan: call get_memory_manager().shutdown_flush(timeout) after channels/scheduler stop, via asyncio.to_thread, try/except bounded. No host-level pending/processing guard -- the backend short-circuits on an idle buffer, so the host cannot "forget" the in-flight case (structurally eliminates the guard race flagged on the prior revision). - shutdown_flush_timeout_seconds added to the shared MemoryConfig (host-owned lifecycle budget, default 30, 1-300) + exposed on MemoryConfigResponse and the embedded client; config_version 25 -> 26. Tests: queue flush_sync (7), lifespan drain incl. False-branch caplog assertion + disabled gate (3), ABC contract noop/deermem (3). * fix(chart): gateway grace period so memory drain is not SIGKILLed K8s defaults terminationGracePeriodSeconds to 30s, shorter than the Gateway's graceful-shutdown work (channel stop ~5s + memory queue drain default 30s). Without an explicit grace period, K8s SIGKILLs the memory drain mid-flight and silently re-introduces the loss shutdown_flush is fixing (flagged on the prior revision). - gateway pod: terminationGracePeriodSeconds (default 45, configurable). - gateway container: preStop sleep (default 5, 0 disables) so the Service/ingress deregisters the pod before SIGTERM begins the drain. - values.yaml + README: both configurable; README documents that the grace period must track memory.shutdown_flush_timeout_seconds. * docs(memory): document shutdown_flush_timeout_seconds + lifespan drain Add the host-shared field to the memory config list and Config Schema summary in backend/AGENTS.md, noting the lifespan drain and the K8s grace-period relationship. * fix(chart): bump embedded config_version to 26 The chart's embedded `config:` block (values.yaml + README example) still had config_version: 25 after commit f3ca8e9f raised config.example.yaml to 26, failing the validate-chart config_version drift check. Bump both to 26. |
||
|
qin-chenghan
|
ad45f59d66
|
feat(memory): pluggable memory abstraction with self-contained DeerMem backend (#4122)
* feat(memory): pluggable + self-contained memory system (MemoryManager plan phases 1 & 2) Phase 1 — Pluggable (steps 0-10): - ABC MemoryManager (9 methods) + singleton factory + drop-in backend discovery - DeerMem default backend with core/ (storage/queue/updater/prompt/message_processing) - NoopMemoryManager backend (proves pluggability) - All call sites (middleware/hook/prompt/gateway/client/app) routed through manager - hasattr capability probing for DeerMem-internal methods (no hard imports) - MemoryConfig gains manager_class field; shared vs DeerMem-private annotated Phase 2 — Self-contained DeerMem (steps 11-18): - backend_config passthrough + DeerMemConfig (all DeerMem-private fields moved off MemoryConfig) - DI: DeerMem owns storage/queue/updater/llm as instance attributes (no global singletons) - Storage independence: core/paths.py with own root (~/.deermem or ), factory auto-injects deer-flow's runtime_home() as absolute base_dir (zero-config) - LLM independence: core/llm.py via langchain init_chat_model (no create_chat_model) - Trace independence: optional tracing_callback replaces inject_langfuse_metadata/request_trace_context - Message processing independence: hide_from_ui default-skip + optional should_keep_hidden_message hook - Internal imports → relative (only deer_mem.py ABC import is host-relative) - Carrier (deer_mem.py adapter) / portable (deermem/ config+core) split - New tests: test_deermem_self_contained + test_memory_manager_pluggable; all memory tests migrated - Other-agent demo: samples/other_agent_demo/ + automated portability test - config.example.yaml memory section updated to phase-2 schema * feat(memory): port consolidation + staleness fix into self-contained DeerMem; phase-2 host hooks Port upstream #3996 (memory consolidation) and #3993 (staleness KeyError fix) from origin/MemoryManager into the pluggable, self-contained DeerMem structure (backends/deermem/deermem/), adapted to the DI MemoryUpdater (config injected, not get_memory_config globals): - DeerMemConfig: add consolidation_enabled (opt-in, default false) / consolidation_min_facts / consolidation_max_groups_per_cycle / consolidation_max_sources - prompt.py: factsToConsolidate JSON field + {consolidation_section} placeholder + CONSOLIDATION_PROMPT constant - updater.py: _coerce_source_confidence / _select_consolidation_candidates / _build_consolidation_section module helpers (matching the existing _select_stale_candidates style); consolidation normalization in _normalize_memory_update_data; consolidation apply in _apply_updates (after max_facts trim, with apply-time guardrails mirroring staleness); staleness KeyError fix (f["id"] -> f.get("id") is not None) applied to both the staleness guardrail and the consolidation allowed_source_ids comprehension - config.example.yaml: consolidation section under memory.backend_config - tests/test_memory_consolidation.py: 40 DI-adapted tests (running, not skipped) incl. the staleness KeyError regression Also includes in-flight phase-2 host-integration work: storage_path semantics (any absolute/relative value = root dir) and host-default tracing_callback / should_keep_hidden_message hooks injected into backend_config by the factory. Co-Authored-By: Claude <noreply@anthropic.com> * feat(memory): add noop backend template and backends guide - backends/noop/: complete drop-in template (config.py with zero deer-flow imports, noop_manager.py with a 6-step new-backend walkthrough in its docstring, commented optional fact-CRUD capabilities). - backends/README.md: which files to touch when adding/swapping a backend, the 5-item backend contract, and common pitfalls. - manager.py: generalize backend examples in comments (drop mem0-specific references). Co-Authored-By: Claude <noreply@anthropic.com> * fix(frontend): guard formatTimeAgo against invalid timestamps Return a neutral placeholder when the input date is invalid (e.g. an empty lastUpdated from a backend with no memories) instead of throwing 'Invalid time value' from date-fns. Co-Authored-By: Claude <noreply@anthropic.com> * feat(memory): wire tool-driven memory mode through the MemoryManager ABC tools.py (memory_search/add/update/delete) now calls get_memory_manager() instead of the removed host memory module, so tool mode (memory.mode: tool) works for any backend. DeerMem.search is implemented (case-insensitive substring match, ranked by confidence) as a stand-in for the planned semantic retrieval; noop.search returns [] (unchanged). Fact-CRUD tools use getattr+callable probing -- backends lacking those ops (noop) get a clear JSON error instead of crashing. Tests: test_memory_tools rewired to mock the manager (handler tests) + TestModeGating retained; test_memory_search now covers DeerMem.search; pluggable stubs test updated (search no longer a stub). Co-Authored-By: Claude <noreply@anthropic.com> * fix: resolve lint errors (import sorting, type annotation quotes, E402 in skipped tests) * docs: restore explanatory comments in config.example.yaml memory section * fix(security): port html-escape memory facts fix (#4097) to vendored DeerMem prompt.py * fix(memory): address review + port dropped upstream memory fixes Review blockers (vendored DeerMem): - #4044 restore _escape_memory_for_prompt (current_memory blob in MEMORY_UPDATE_PROMPT) - prevents </current_memory> breakout - #4028 html.escape staleness-section cat/content in _build_staleness_section - #4119 add _escape_summary for injection-path summaries (Work/Personal/ Current Focus/Recent/Earlier/Background) - default-model silent no-op: factory injects host default chat model via a new host_llm slot (create_chat_model(name=None)); DeerMem prefers host_llm over build_llm(model). Zero-config extraction works out of the box again - MemoryConfigResponse: fix stale docstring (backend-agnostic shape; DeerMem knobs live under backend_config, not top-level - restoring flat would re-couple the API to DeerMem). Frontend audited: does not read /memory/config - _host_default_tracing_callback: restore langfuse assistant_id/environment - search: push category onto the ABC signature; DeerMem filters BEFORE the top_k slice (was filtered client-side after slicing -> starved results) - _do_update_memory_sync: split into wrapper+impl; bind trace_id into the request-trace ContextVar on the Timer/executor worker via a new trace_context_manager host hook (None trace_id left unbound - no fabrication) - client.py fact-CRUD now passes user_id (was writing to the global bucket while get_memory reads per-user) - _resolve_manager_class: fail-fast (raise ValueError) on an unresolved explicit manager_class instead of silently falling back to DeerMem (memory is persistent state - a wrong store is a silent data-integrity footgun) Upstream memory fixes dropped by the host->vendored rename conflict, re-ported to backends/deermem/deermem/core/ (+ deer_mem.py): - #4073 queue busy-timer-spin -> _reprocess_pending flag (core/queue.py) - #4074 null source.confidence in staleness -> _coerce_source_confidence (core/updater.py: _build_staleness_section + _apply_updates stale sort) - #4075 factsToRemove is optional (drop from _REQUIRED_MEMORY_UPDATE_TOP_LEVEL_KEYS) - #4076 null confidence in search ranking -> _coerce_source_confidence (deer_mem.py DeerMem.search) host_llm + trace_context_manager are host-injected via backend_config (factory in manager.py), keeping backends/deermem/ at exactly one `from deerflow` line (the ABC contract) - portability test preserved. Co-Authored-By: Claude <noreply@anthropic.com> * fix: resolve lint errors (F541 f-string without placeholders, E501 line too long) * fix(memory): restore hide_from_ui clarification preservation, expose mode Two memory-system fixes (F541/E501 lint was already fixed on this branch): - filter_messages_for_memory: restore default preservation of well-formed human_input_response clarification answers (v2 regression). The self-containment refactor made the bare function skip ALL hide_from_ui when no hook was passed, but upstream preserves well-formed clarification responses by default (test_hide_from_ui_human_input_response_is_preserved). Inline a host-agnostic _is_human_clarification_response mirror of read_human_input_response as the default keep-decision; the host-injected should_keep_hidden_message hook still overrides (production path unchanged). Portable package stays zero `from deerflow`. - /memory/config: expose `mode` (middleware|tool) in MemoryConfigResponse + the config/status endpoints + client.get_memory_config. mode is a host- shared, behavior-determining field missing from the response projection. Sync tests (mock .mode; e2e assert mode present). - Align manager_class field docstring with fail-fast behavior. Tests: filter/self-contained/portability (35) + memory-config (4) pass; ruff clean. Co-Authored-By: Claude <noreply@anthropic.com> * fix(memory): resolve ruff format failures in memory module + tests `make lint` runs `ruff format --check` in addition to `ruff check`; 8 memory files had pending format changes -- 7 pre-existing (deer_mem, updater, tools, test_memory_queue/router/search/tools) + message_processing from the hide_from_ui fix. Apply `ruff format`: whitespace/wrapping only, no logic change. 109 memory tests pass; ruff check + format --check both clean. Co-Authored-By: Claude <noreply@anthropic.com> * fix(memory): address PR review - legacy field migration, fact_id contract, path/docs Address willem-bd's review on PR head bc8bf0d4 (risk:high, persistent state): - config: auto-migrate pre-abstraction top-level memory.* DeerMem fields (storage_path, max_facts, debounce_seconds, model_name, token_counting, staleness_*, consolidation_*) into backend_config on load + warn, so an upgrade does NOT silently revert customized settings (was: silent extra='ignore' drop). model_name -> backend_config.model.model. Unknown top-level keys warned. - factory: resolve a relative backend_config.storage_path against runtime_home() (base_dir-relative, CWD-independent) to preserve pre-abstraction semantics; paths.py stays portable (no runtime_home import). - tools: memory_add uses the fact_id returned directly by create_fact instead of re-deriving it via content-key matching (coupled the tool to the backend's content normalization; could misreport a storage cap). create_fact now returns (memory_data, fact_id); gateway/client/tool updated. Fix terse {"error":"content"} -> {"error":"empty content"}. - app.py: update stale token_counting=="char" warm-up comment to point at manager.warm (DeerMem.warm re-checks char and returns early). - router: comment explaining reload_memory silent fallback vs fact 501 asymmetry (read-only degrade vs write fail-loud). - CHANGELOG: document breaking changes (/memory/config + client.get_memory_config shape flat->backend_config; custom storage_class path moved + __init__ must accept config) and the legacy-field auto-migration. - tests: add regression test pinning the per-user memory path ({storage_path}/users/{safe_user_id}/memory.json == host make_safe_user_id) across the abstraction; update create_fact mocks for (memory_data, fact_id). Tests: 273 passed (memory suite); ruff check + format clean. Co-Authored-By: Claude <noreply@anthropic.com> * fix(memory): address PR review - storage_path, max_facts, tracing, parsing Six review findings (willem-bd), each verified against upstream: - storage_path semantics (file -> root dir): migration drops file-style (.json) legacy values with a warning; factory raises if storage_path resolves to an existing file (avoid silent NotADirectoryError write failure). CHANGELOG + config.example.yaml comment updated. - create_memory_fact enforces max_facts again (via _trim_facts_to_max) and returns (memory, None) when the cap evicts the new fact; memory_add tool reports "not stored", client raises ValueError, POST /memory/facts -> 409. - max_facts trim uses _coerce_source_confidence (was raw f.get("confidence", 0) -> TypeError on non-float imported/legacy confidence, swallowed as silent update failure). - memory-tracing assistant_id restored to "memory_agent" (was "lead-agent" copy-paste; matches upstream + DeerMem run_name). - _is_human_clarification_response cross-checked against read_human_input_response (drift guard test). - empty-string legacy values skipped silently in migration (narrow fix, not broad "if not value" which would skip explicit bool False). 8 new regression tests. make lint + 406 memory tests pass. Co-Authored-By: Claude <noreply@anthropic.com> * fix(memory): address internal review - storage fail-fast, build_llm degrade, config warn, noop template Addresses 4 findings from the PR #4122 internal supplemental review (parallel to willem-bd's review, no overlap): - create_storage fail-fast: a misspelled/unimportable storage_class now raises ValueError instead of silently falling back to FileMemoryStorage. Memory is persistent state, so a wrong store is a data-integrity footgun; mirrors the existing manager_class resolution policy. (storage.py) - noop template create_fact signature: the commented template used keyword-only `content` and returned a bare dict, while DeerMem's actual create_fact takes positional `content` and returns tuple[dict, str|None] (the memory_add tool passes content positionally; gateway/client/tools all tuple-unpack). A backend copied from the template would 500 on fact-CRUD. Template fixed; delete_fact/update_fact templates left (callers compatible). (noop_manager.py) - build_llm graceful degrade: wrap init_chat_model in try/except, degrade to None + WARNING on failure (mirroring _host_default_llm) so a misconfigured explicit model does not crash app startup -- non-LLM memory ops still work and an update raises at runtime with the error logged. (llm.py) - from_backend_config unknown-key warning: log a WARNING for unknown backend_config keys (mirrors the host layer's load_memory_config_from_dict) so a typo like `storage_pat` does not silently fall back to the default and write memory to an unintended location. (config.py) Tests: rewrote 3 create_storage fallback tests to expect ValueError; added 4 tests (build_llm zero-config/degrade, from_backend_config warn/silent). make lint green; full memory suite passes. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: lllyfff <2281215061@qq.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: lllyfff <122260771+lllyfff@users.noreply.github.com> |
||
|
DanielWalnut
|
25ea6970a6
|
feat(runtime): implement goal continuations (#3858)
* implement goal continuations * fix(goal): address review findings for goal continuations - goal: key the no-progress breaker on a signature of the latest visible assistant evidence instead of the evaluator's volatile free-text, so it actually fires on stalled turns; thread the signature through every worker persist / no-progress call site - goal: align _stand_down_reason default caps with should_continue_goal (8 / 2) so the two gate functions agree on goals missing the fields - runtime: offload the synchronous checkpointer fallback via asyncio.to_thread (goal.py + worker.py) to keep blocking IO off the loop - frontend: i18n the GoalStatus "Goal" label (goalLabel in en/zh/types) - frontend: extract pure composer helpers into input-box-helpers.ts with unit tests (parseGoalCommand, readGoalResponseError, skill suggestions) - tests: cover the evidence-based no-progress and default-cap behavior - docs: align backend/AGENTS.md goal paragraph with actual behavior - e2e: prettier-format chat.spec.ts (fixes the lint-frontend CI failure) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(frontend): hide goal continuation counter until the agent continues The goal status bar rendered a raw "0/8" before any auto-continuation, which read as a mysterious score. Now the counter is hidden until continuation_count > 0, then shows "Continuing N/M" with a tooltip explaining the auto-continuation cap. - Extract getGoalContinuationDisplay into a pure helper (hides at 0) + unit tests - Add goalContinuing / goalContinuationTooltip i18n keys (en/zh/types) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(goal): address review findings for goal continuations Frontend correctness - Fix the optimistic /goal result permanently shadowing server goal state: the streamed continuation counter never surfaced for a goal set in-session. Extract a shared useActiveGoal hook (used by both chat pages) that reconciles the optimistic copy with server state via a goalReconciliationKey, de-duping the copy-pasted goal block across the two pages. - Stop /goal status|clear failures from escaping handleSubmit as unhandled rejections (handleGoalCommand now returns success; the run only starts when a goal was actually saved). - Use a function replacer for the goal-status toast so an objective containing $&/$1 isn't treated as a replacement pattern. Backend cleanliness / correctness - De-duplicate four byte-identical helpers (_call_checkpointer_method, _message_type, _additional_kwargs, _is_visible_message) by importing them from runtime.goal instead of re-defining them in the run worker. - Remove the dead `checkpoint_tuple.tasks` durability guard (CheckpointTuple has no tasks field) and document that pending_writes is the durability signal. - Decompose the 176-line _prepare_goal_continuation_input: extract _reread_goal_and_checkpoint and a _persist closure so the thread-unchanged guard and stand-down persistence aren't open-coded three times. Document the last-writer-wins write-window limitation as a follow-up. - Add a shared parse_goal_command helper and use it from the TUI and IM-channel /goal handlers (one place for the status/clear/set semantics). Tests - Restore the 11 command-registry tests dropped by the previous goal change (filter_commands ranking/description, build_registry builtins/skills, resolve cases) alongside the new goal tests. - Add coverage for the IM-channel _handle_goal_command, the TUI _handle_goal handler, parse_goal_command, and goalReconciliationKey. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix goal review feedback * fix goal continuation checkpoint races * prioritize goal commands while streaming Route composer submits through a shared helper so /goal commands can be handled before the streaming stop shortcut, while ordinary streaming submits still stop the active run. Testing: cd frontend && pnpm exec rstest run tests/unit/components/workspace/input-box-helpers.test.ts tests/unit/components/workspace/goal-status-helpers.test.ts; cd frontend && pnpm check * preserve goal status during clarification Keep omitted stream goal fields distinct from explicit null clears so clarification interrupts do not hide an active thread goal that is still present in the checkpoint. Testing: pnpm exec rstest run tests/unit/components/workspace/use-active-goal.test.ts tests/unit/components/workspace/input-box-helpers.test.ts tests/unit/components/workspace/goal-status-helpers.test.ts; pnpm check; git diff --check * style: format active goal hook Run Prettier on use-active-goal.ts to satisfy the frontend lint workflow formatting gate. Testing: pnpm format; pnpm exec rstest run tests/unit/components/workspace/use-active-goal.test.ts tests/unit/components/workspace/input-box-helpers.test.ts tests/unit/components/workspace/goal-status-helpers.test.ts; pnpm check; git diff --check * fix goal review followups --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Minh Vu
|
8fa6ed2b54
|
fix(gateway): fix oversized upload replacements deleting existing files (#3822)
* fix upload rollback on size limit * fix upload staging cleanup and listings |
||
|
Zheng Feng
|
ee8ad1bc67
|
feat(auth): add OIDC SSO support (#3506)
Add provider-agnostic OIDC authentication with Keycloak-compatible configuration, frontend SSO UI support |
||
|
Xinmin Zeng
|
e93f658472
|
fix(stability): resolve P0 blockers from v2.0-m1-rc1 stability audit (#3107) (#3131)
* fix(task-tool): unwrap callback manager when locating usage recorder
`config["callbacks"]` may arrive as a `BaseCallbackManager` (e.g. the
`AsyncCallbackManager` LangChain hands to async tool runs), not just a plain
list. The previous `for cb in callbacks` loop raised
`TypeError: 'AsyncCallbackManager' object is not iterable`, which
`ToolErrorHandlingMiddleware` then converted into a failed `task` ToolMessage
even though the subagent had completed internally — Ultra mode lost subagent
results and the lead agent fell back to redoing the work.
Unwrap `BaseCallbackManager.handlers` before searching for the recorder.
Refs: bytedance/deer-flow#3107 (BUG-002)
* fix(frontend): treat any task tool error as a terminal subtask failure
The subtask card status machine matched only three English prefixes (`Task
Succeeded. Result:`, `Task failed.`, `Task timed out`). Anything else fell
through to `in_progress`, so a `task` tool error wrapped by
`ToolErrorHandlingMiddleware` (`Error: Tool 'task' failed ...`) left the card
spinning forever even after the run had ended.
Extract the prefix logic into `parseSubtaskResult` and recognise any leading
`Error:` token as a terminal failure. The extracted function is unit-tested
against the legacy prefixes plus the `AsyncCallbackManager` regression
captured in the upstream issue.
Refs: bytedance/deer-flow#3107 (BUG-007)
* fix(frontend): exclude hidden, reasoning, and tool payloads from chat export
`formatThreadAsMarkdown` / `formatThreadAsJSON` iterated raw messages without
running the UI-level `isHiddenFromUIMessage` filter. Exported transcripts
therefore included `hide_from_ui` system reminders, memory injections,
provider `reasoning_content`, tool calls, and tool result messages — content
that is intentionally hidden in the chat view.
Filter the export to the user-visible transcript by default and gate
reasoning / tool calls / tool messages / hidden messages behind explicit
`ExportOptions` flags so a future debug export can opt back in without
forking the formatter.
Refs: bytedance/deer-flow#3107 (BUG-006)
* fix(gateway): route get_config through get_app_config for mtime hot reload
`get_config(request)` returned the `app.state.config` snapshot captured at
startup. The worker / lead-agent path then threaded that frozen `AppConfig`
through `RunContext` and `agent_factory`, so per-run fields edited in
`config.yaml` (notably `max_tokens`) were ignored until the gateway process
was restarted — even though `get_app_config()` already does mtime-based
reload at the bottom layer.
Route the request dependency through `get_app_config()` directly. Runtime
`ContextVar` overrides (`push_current_app_config`) and test-injected
singletons (`set_app_config`) keep working; `app.state.config` is now only
read at startup for one-shot bootstrap (logging level, IM channels,
`langgraph_runtime` engines).
`tests/test_gateway_deps_config.py` encoded the old snapshot contract and is
removed; `tests/test_gateway_config_freshness.py` replaces it with mtime,
ContextVar, and `set_app_config` coverage. `test_skills_custom_router.py` and
`test_uploads_router.py` now inject test configs via FastAPI
`dependency_overrides[get_config]` instead of mutating `app.state.config`.
Document the hot-reload boundary in `backend/CLAUDE.md` so reviewers know
which fields are picked up on the next request vs. which still require a
restart (`database`, `checkpointer`, `run_events`, `stream_bridge`,
`sandbox.use`, `log_level`, `channels.*`).
Refs: bytedance/deer-flow#3107 (BUG-001)
* fix(gateway): broaden get_config 503 to any config-load failure
Address review feedback on the previous commit:
1. Narrow exception catch removed. The old contract returned 503 whenever
`app.state.config is None`. The first cut only mapped
`FileNotFoundError`, leaving `PermissionError`, YAML parse errors, and
pydantic `ValidationError` to bubble up as 500. At the request boundary
we treat any inability to materialise the config as "configuration not
available" (503) and log the original exception so the operator still
has the stack.
2. Removed the unused `request: Request` parameter and the matching
`# noqa: ARG001`. FastAPI's `Depends()` does not require the dependency
to accept `Request`; the only call site uses the no-arg form.
3. `backend/CLAUDE.md` boundary now lists the *reason* each field is
restart-required (engine binding, singleton caching, one-shot
`apply_logging_level`, etc.), not just the field name, so reviewers do
not have to reverse-engineer the boundary themselves.
Tests parametrise four exception classes (`FileNotFoundError`,
`PermissionError`, `ValueError`, `RuntimeError`) and assert 503 for each.
Refs: bytedance/deer-flow#3107 (BUG-001)
* fix(task-tool): defend _find_usage_recorder against non-list callbacks
Address review feedback. The previous commit handled the two common shapes
LangChain hands to async tool runs — a plain `list[BaseCallbackHandler]` and
a `BaseCallbackManager` subclass — but iterated any other shape directly,
which would still raise `TypeError` if e.g. a single handler instance leaked
through without a list wrapper.
Treat any non-list, non-manager `config["callbacks"]` value as "no recorder"
rather than crash. Docstring now lists all four shapes explicitly. New tests
cover the single-handler-object case, `runtime is None`, `callbacks is None`,
and `runtime.config` being a non-dict — all required to be silent no-ops.
Refs: bytedance/deer-flow#3107 (BUG-002)
* fix(frontend): drop dead identity ternary and add opt-in export tests
Address review feedback on the previous export commit:
1. Removed the no-op `typeof msg.content === "string" ? msg.content : msg.content`
expression in `formatThreadAsJSON`. Both branches returned the same value;
the message content now flows through unchanged whether it is a string or
the rich `MessageContent[]` shape (LangChain JSON-serialises the array
structure correctly already).
2. Expanded the JSDoc on `ExportOptions` to make it clearer that the four
flags are not currently wired to any UI control — callers wanting a debug
export must build the options object explicitly. The default behaviour
continues to match the explicit prescription in
bytedance/deer-flow#3107 BUG-006.
3. Added opt-in coverage. The previous tests only exercised the
`options = {}` default path; the new cases verify each flag flips the
corresponding payload back into the export so a future debug-export
surface does not silently break the contract.
Refs: bytedance/deer-flow#3107 (BUG-006)
* fix(frontend): export subtask prefix constants and document fallback intent
Address review feedback on the previous BUG-007 commit:
1. `SUCCESS_PREFIX`, `FAILURE_PREFIX`, `TIMEOUT_PREFIX`, and the
`ERROR_WRAPPER_PATTERN` regex are now exported. The JSDoc explicitly
pins them as part of the backend↔frontend contract defined in
`task_tool.py` and `tool_error_handling_middleware.py`, so any future
structured-status migration (e.g. backend writing
`additional_kwargs.subagent_status` instead of leading text) can
reference these from one canonical place rather than redefine them.
2. The `in_progress` fallback now carries a docstring explaining the
deliberate choice — LangChain only ever emits a `ToolMessage` once the
tool itself has returned, so unrecognised content means the contract
has drifted and "still running" is the right operator signal (eagerly
marking it terminal-failed would mask the drift).
No behaviour change; this is documentation and an API export.
Refs: bytedance/deer-flow#3107 (BUG-007)
* fix(gateway): drop app.state.config snapshot and freeze run_events_config
Address @ShenAC-SAC's BUG-001 review on #3131. The previous cut still
stored an ``AppConfig`` snapshot on ``app.state.config`` for startup
bootstrap. Two follow-on hazards from that:
1. Future code touching the gateway lifespan could accidentally start
reading ``app.state.config`` again, silently regressing the request
hot path back to a stale snapshot.
2. ``get_run_context()`` paired a freshly-reloaded ``AppConfig`` with the
startup-bound ``event_store`` and a *live* ``run_events_config``
field — so an operator who edited ``run_events.backend`` mid-flight
would have produced a run context whose ``event_store`` and
``run_events_config`` referred to different backends.
Clean approach (aligned with the direction in PR #3128):
- ``lifespan()`` keeps a local ``startup_config`` variable and passes it
explicitly into ``langgraph_runtime(app, startup_config)`` and into
``start_channel_service``. No ``app.state.config`` attribute is set at
any point.
- ``langgraph_runtime`` now accepts ``startup_config`` as a required
parameter, removing the ``getattr(app.state, "config", None)`` lookup
and the "config not initialised" runtime error.
- The matching ``run_events_config`` is frozen onto ``app.state`` next
to ``run_event_store`` so ``get_run_context`` reads the two from the
same startup-time source. ``app_config`` continues to be resolved
live via ``get_app_config()``.
- ``backend/CLAUDE.md`` boundary explanation updated to spell out the
``startup_config`` / ``get_app_config()`` split.
New regression test ``test_run_context_app_config_reflects_yaml_edit``
exercises the worker-feeding path: it asserts that ``ctx.app_config``
follows a mid-flight ``config.yaml`` edit while
``ctx.run_events_config`` stays frozen to the startup snapshot the
event store was built from.
Refs: bytedance/deer-flow#3107 (BUG-001), bytedance/deer-flow#3131 review
* fix(frontend): parse Task cancelled and polling timed out as terminal
Address @ShenAC-SAC's BUG-007 review on #3131. `task_tool.py` actually
emits five terminal strings:
- `Task Succeeded. Result: …`
- `Task failed. …`
- `Task timed out. …`
- `Task cancelled by user.` ← previously matched none
- `Task polling timed out after N minutes …` ← previously matched none
The previous cut handled three; the last two fell through to the
"unknown content" branch and pushed the subtask card back to
`in_progress` even though the backend had already reached a terminal
state. Add explicit matches plus regression tests for both. The
`in_progress` fallback is now reserved for genuinely unrecognised
output (i.e. contract drift), as documented.
Refs: bytedance/deer-flow#3107 (BUG-007), bytedance/deer-flow#3131 review
* fix(frontend): sanitize JSON export content via the Markdown content path
Address @ShenAC-SAC's BUG-006 review and the Copilot inline comment on
#3131. The previous cut filtered hidden/tool messages out of the JSON
export but still serialised `msg.content` verbatim, so:
- inline `<think>…</think>` wrappers stayed in the exported `content`
even with `includeReasoning: false`,
- content-array thinking blocks leaked the `thinking` field,
- `<uploaded_files>…</uploaded_files>` markers leaked the workspace
paths a user uploaded files to.
JSON now goes through the same sanitiser the Markdown path uses
(`extractContentFromMessage` + `stripUploadedFilesTag`). Reasoning and
tool_calls remain gated behind their `ExportOptions` flags. AI / human
rows that sanitise to empty content with no opted-in reasoning or tool
calls are dropped so the JSON matches the Markdown path's `continue`
on empty assistant fragments.
New regression tests cover the three leak shapes the reviewer called
out plus the empty-content-drop case.
Refs: bytedance/deer-flow#3107 (BUG-006), bytedance/deer-flow#3131 review
* test(gateway): align lifespan stub with langgraph_runtime two-arg signature
Codex round-3 review of c0bc7a06 flagged this: changing
`langgraph_runtime` to require `startup_config` as a second positional
argument broke the one-arg stub `_noop_langgraph_runtime(_app)` in
`test_gateway_lifespan_shutdown.py`, which is patched into
`app.gateway.app.langgraph_runtime` by the lifespan shutdown bounded-timeout
regression. Lifespan would then call the stub with two args and raise
`TypeError` before the bounded-shutdown assertion ran.
Update the stub to match the new signature. The shutdown test itself is
unaffected — it only cares about the channel `stop_channel_service` hang
path.
Refs: bytedance/deer-flow#3107 (BUG-001), bytedance/deer-flow#3131 review
* fix(frontend): strip every known backend marker in export, not just uploads
Codex round-3 review of 258ca800 and the matching maintainer feedback on
PR #3131 made the same point: the JSON export now ran the
Markdown-side sanitiser, but that sanitiser only stripped
`<uploaded_files>`. The full set of payloads middleware embeds inside
message `content` is larger:
- `<uploaded_files>` — `UploadsMiddleware`
- `<system-reminder>` — `DynamicContextMiddleware`
- `<memory>` — `DynamicContextMiddleware` (nested inside system-reminder)
- `<current_date>` — `DynamicContextMiddleware`
The primary protection is still `isHiddenFromUIMessage`: the
`<system-reminder>` HumanMessage is marked `hide_from_ui: true` and never
reaches the formatter. This commit adds the second line of defence so a
regression that drops the `hide_from_ui` flag — or any future middleware
that injects the same tag vocabulary into a visible HumanMessage —
cannot leak the payload into the export file.
Concrete changes:
- New `INTERNAL_MARKER_TAGS` constant + `stripInternalMarkers(content)`
helper in `core/messages/utils.ts`. The constant doubles as
documentation for the backend↔frontend contract.
- `formatMessageContent` in `export.ts` now calls `stripInternalMarkers`
instead of `stripUploadedFilesTag`. UI render paths
(`message-list-item.tsx`) keep using the narrower function so a user
legitimately typing `<memory>` in a meta-discussion is preserved.
- The "drop empty rows" guard in `buildJSONMessage` switched from
`=== undefined` to truthy `!` checks. Codex spotted the asymmetry: when
`extractReasoningContentFromMessage` returned the empty string (which it
legitimately can), the JSON path emitted `{reasoning: ""}` while the
Markdown path's `!reasoning` `continue` correctly dropped the row.
New regression tests cover the defence-in-depth strip with a
`<system-reminder><memory><current_date>` payload deliberately *not*
marked `hide_from_ui`; tool-message sanitization under
`includeToolMessages: true`; the mixed-content-array case
(`thinking + text + image_url`); and the opted-in empty-reasoning drop.
Live verification on a real Ultra-mode thread that uploaded a PDF
(`曾鑫民-薪资交易流水.pdf`): backend state's first HumanMessage carries the
`<uploaded_files>` block (with `/mnt/user-data/uploads/...` paths) as part
of a content-array. The Markdown and JSON export blobs both come back
free of `<uploaded_files>`, `<system-reminder>`, `<current_date>`,
`tool_calls`, and reasoning — while preserving the user's `这是什么 ？`
prompt and the assistant's visible answer.
Refs: bytedance/deer-flow#3107 (BUG-006), bytedance/deer-flow#3131 review
* test(frontend): cover trim, varied N, and pre-execution Error: prefixes
Codex round-3 review of 50e2c257 flagged three coverage gaps in the
subtask-status parser:
1. `Task cancelled by user.` and `Task polling timed out` previously had
no whitespace-trim coverage — the original trim test only exercised
the success prefix. Streaming chunks can arrive with leading/trailing
newlines; the regex needed an explicit assertion.
2. The polling-timeout case was tested only at one `N` (15 minutes). The
backend interpolates the live `timeout_seconds // 60` value, so the
matcher must hold for any positive integer. Now we run the case for
1, 5, and 60 minutes.
3. `task_tool.py` also emits three `Error:` strings for pre-execution
failures — unknown subagent type, host-bash disabled, and "task
disappeared from background tasks". They are intentionally handled by
`ERROR_WRAPPER_PATTERN` rather than dedicated prefixes (the wrapper
already produces the right terminal-failed shape) but had no test
coverage proving that wiring. Codex was right that a refactor splitting
one of them off into its own prefix would silently break things.
The JSDoc on the constants block now spells the three pre-execution
errors out so the relationship between `task_tool.py` returns and the
prefix vocabulary is explicit.
No production code change beyond the docstring — this commit is pure
coverage hardening for the contract that already exists.
Refs: bytedance/deer-flow#3107 (BUG-007), bytedance/deer-flow#3131 review
|
||
|
JerryChaox
|
4e72410154
|
fix(gateway): bound lifespan shutdown hooks to prevent worker hang under uvicorn reload (#2331)
* fix(gateway): bound lifespan shutdown hooks to prevent worker hang Gateway worker can hang indefinitely in `uvicorn --reload` mode with the listening socket still bound — all /api/* requests return 504, and SIGKILL is the only recovery. Root cause (py-spy dump from a reproduction showed 16+ stacked frames of signal_handler -> Event.set -> threading.Lock.__enter__ on the main thread): CPython's `threading.Event` uses `Condition(Lock())` where the inner Lock is non-reentrant. uvicorn's BaseReload signal handler calls `should_exit.set()` directly from signal context; if a second signal (SIGTERM/SIGHUP from the reload supervisor, or watchfiles-triggered reload) arrives while the first handler holds the Lock, the reentrant call deadlocks on itself. The reload supervisor keeps sending those signals only when the worker fails to exit promptly. DeerFlow's lifespan currently awaits `stop_channel_service()` with no timeout; if a channel's `stop()` stalls (e.g. Feishu/Slack WebSocket waiting for an ack), the worker can't exit, the supervisor keeps signaling, and the deadlock becomes reachable. This is a defense-in-depth fix — it does not repair the upstream uvicorn/CPython issue, but it ensures DeerFlow's lifespan exits within a bounded window so the supervisor has no reason to keep firing signals. No behavior change on the happy path. Wraps the shutdown hook in `asyncio.wait_for(timeout=5.0)` and logs a warning on timeout before proceeding to worker exit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Update backend/app/gateway/app.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * style: apply make format (ruff) to test assertions Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> |