mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-07-21 10:15:47 +00:00
b3a0dac8ad191fbfa248c0a1cecc8f2deeaa1c7d
2643
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 AariandGitHub
|
b3a0dac8ad |
fix(channels): accept leading @mentions before /connect bind codes (#4222)
* fix(channels): accept leading @mentions before /connect bind codes Group chats often deliver "@bot /connect <code>" (Feishu/DingTalk leave the mention in the text). extract_connect_code required the message to start with /connect, so those binds silently failed while Slack/Discord already strip mentions before parsing. Skip leading mention tokens in the shared helper. * test(channels): pin mention variants and case-insensitive /connect parsing |
||
|
|
94a34f382d |
feat(context): record effective memory identity per run (#3556)
* feat(context): record effective memory identity per run * fix(context): address memory identity review feedback --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Tianye SongandGitHub
|
8da7cbf028 |
feat(memory): LLM-assigned per-fact expected_valid_days and staleFactsToExtend (#4143)
Re-ports this feature onto the pluggable-memory backend introduced in #4122 (the original #4143 was force-pushed clean by accident and auto-closed). The #4122 refactor moved the staleness logic into the self-contained DeerMem backend (backends/deermem/deermem/core/) and reverted it to the pre-feature global-threshold version, so the per-fact lifetime work is re-applied here against the new module layout + DI (MemoryUpdater is now (config, storage, llm)-injected; config lives on DeerMemConfig, not host MemoryConfig). **expected_valid_days (creation)** The LLM assigns a per-fact review window when storing each new fact. The prompt exposes five tiers (<=14 d transient -> >365 d very stable). The value is capped at write time by staleness_age_days x staleness_max_lifetime_multiplier (default 20.0 -> 1800 d ~= 5 years; range 1.0-100.0) so the model cannot set an initial lifetime so long the fact is never re-evaluated. The default 20.0 makes the "> 365 d very stable" tier achievable out of the box (3.0 silently clamped it to 270 d). **staleFactsToExtend (review)** During staleness review the LLM can emit extension entries for kept facts whose window seems miscalibrated. new_evd = min(days_since_created + extend_by_days, staleness_max_extension_days). Extensions use an absolute ceiling (default 3650 d ~= 10 years; range 90-36500) rather than the creation multiplier - they are deliberate review decisions that must be able to advance the window beyond the initial cap, but the absolute bound prevents timedelta overflow (a model-supplied extend_by_days of 10**9 previously crashed every later candidate-selection pass with OverflowError) and LLM misfire. **Invariant correctness** - Read-time cap removed from _effective_fact_staleness_age; cap is write-time only so extensions actually advance the review window. - proposed_remove_ids hoisted out of the removals sub-block and used to exclude from extension, so a cap-surviving proposed-removal fact is never extended. - extend_by coerced to int before the > 0 guard (a fractional 0.9 would pass the float check then int() to 0, silently writing a zero-delta extension). - days_since uses total_seconds() // 86400 (not .days truncation). - staleness-section html.escape uses quote=False to match the prompt.py convention; only <, >, & break element-text structure. **Tests** test_memory_staleness_review.py was module-level skipped by #4122 ("full unit-test migration is a follow-up"). This PR performs that migration: DI construction via (DeerMemConfig, _FakeStorage), _build_staleness_section back to the (candidates, config) signature, plus new coverage for per-fact selection, EXTEND with the absolute cap, the overflow next-cycle regression, the proposed-removal-not-extendable case, fractional extend_by skipping, and the creation-time cap. 67 tests, all green. |
||
|
AariandGitHub
|
259f51ca4f |
fix(github): match allow_authors logins case-insensitively (#4218)
* fix(github): match allow_authors logins case-insensitively GitHub logins are case-insensitive, and the sibling gates in this module already treat them that way. allow_authors used a bare string membership test, so a YAML casing mismatch silently dropped owner webhooks that should have bypassed require_mention. * test(github): parametrize allow_authors case-fold over both directions Cover the reverse cfg "alice" / payload "Alice" direction plus the all-caps and exact-case rows from the PR's E2E matrix. Each casing-differs row is red on the pre-fix source; the exact-case row stays green both ways, pinning that case-folding is a superset of the old exact match. |
||
|
|
5c80c07dfe |
fix(memory): treat explicit null backend_config values as omitted in DeerMemConfig (#4217)
config.example.yaml ships backend_config.model: as a bare key whose children
are all comments, which YAML parses to None (make config-upgrade then writes
an explicit model: null). DeerMemConfig.model is a non-Optional field with a
default, so from_backend_config(**{"model": None}) raised a ValidationError
and every run failed with "Input should be a valid dictionary or instance of
DeerMemModelConfig". Drop None entries in from_backend_config so YAML null /
empty keys fall back to field defaults, matching the documented "empty =
host default LLM" semantics. Upstream bug (#4122 schema); regression-pinned
in test_deermem_self_contained.py.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
||
|
VanzerenandGitHub
|
1769b2de0d |
fix: read run stop_reason from runtime context (#4188)
* fix: read run stop_reason from runtime context * fix: address review feedback for #4188 stop_reason integration - migration 0005: use safe_add_column for consistency and drift detection - worker: clear runtime.context stop_reason at start of each _stream_once turn so a clean continuation doesn't inherit a prior cap reason - tests: replace circular unit test with real middleware integration tests that exercise LoopDetectionMiddleware._apply and TokenBudgetMiddleware._apply through the worker, proving the full middleware → runtime.context → persist pipeline * fix(test): resume conftest * fix: stamp stop_reason in all guard middlewares, fix clearing semantics |
||
|
Nan GaoandGitHub
|
de55982c5a |
fix(subagents): preserve parent checkpoint namespace (#4215)
* fix(subagents): preserve parent checkpoint namespace * test(subagents): align stream isolation coverage |
||
|
Daoyuan LiandGitHub
|
45865e9f3f |
fix(tracing): attach Langfuse trace metadata to the goal evaluator (#4202)
The goal evaluator (runtime/goal.py) runs from runtime/runs/worker.py after
the main graph run has already completed, so there is no graph root for it
to inherit tracing from. create_goal_evaluator_model was built with
attach_tracing=False, and evaluate_goal_completion invoked the model with a
bare config={"run_name": "goal_evaluator"} — no tracing callbacks, no
Langfuse session/user attribution. Every goal-evaluator LLM call went
untraced.
Same class of gap fixed by #2944 for the main agent graph and by #3902 for
memory_agent/suggest_agent: a standalone call site that invokes a model
directly instead of through a traced graph root must attach its own tracing
callbacks and inject Langfuse trace-attribute metadata itself.
- create_goal_evaluator_model: attach_tracing=False -> True, matching the
other standalone non-graph callers (oneshot_llm.run_oneshot_llm,
MemoryUpdater).
- evaluate_goal_completion: accept optional thread_id/user_id/
deerflow_trace_id and inject Langfuse trace metadata onto the ainvoke
config via the shared inject_langfuse_metadata() helper, mirroring
oneshot_llm.py's pattern.
- worker.py: thread user_id (resolve_runtime_user_id(runtime)) and
deerflow_trace_id through _prepare_goal_continuation_input into
evaluate_goal_completion so the evaluator's trace groups under the
triggering run's thread/session.
Updates the existing test that pinned attach_tracing=False as expected
behavior, and adds a regression test asserting the ainvoke config carries
Langfuse trace metadata when enabled.
|
||
|
Daoyuan LiandGitHub
|
6ef0aa2aea |
fix(tui): stop empty-id assistant deltas from merging across turns (#4201)
_apply_assistant_delta matched AssistantDelta.id anywhere in the transcript, which is correct for a genuine per-message id but not for an empty one. runtime._as_str() coerces a missing/None chunk id to "", and that value is shared by every id-less chunk from every turn, not just the current one. Once one assistant row had id="", every later, unrelated AssistantDelta that also carried id="" (e.g. from a provider that never stamps per-chunk ids) matched that same stale row instead of starting a fresh one, silently folding a second turn's answer backward into the first turn's bubble. Route empty-id deltas to a dedicated path that tracks the current turn's row by position (streaming_anonymous_row_index, reset on RunStarted/ RunEnded/ClearRows) instead of by id, mirroring the existing empty-id guards in _apply_tool_started/_apply_tool_result but adapted for assistant text: unlike a tool call, an id-less assistant delta still needs to be displayed, so it starts a new row rather than being dropped. Multiple id-less chunks legitimately arrive within one turn (per-token streaming), so they keep coalescing into that row -- but only while it is still the transcript tail; once a tool card is appended after it (the same way a genuine id naturally changes across a tool round-trip), the next empty-id delta starts fresh instead of reaching backward past the tool card. Add regression coverage for the cross-turn merge, same-turn coalescing, the tool-call-interleaved edge case, and non-interference with the existing id-keyed path. |
||
|
Huixin615andGitHub
|
3247f61750 |
fix(middleware): sanitize invalid tool call arguments (#4193)
* fix(middleware): sanitize invalid tool call arguments * refactor(middleware): share tool argument parsing |
||
|
|
289adcbb02 |
fix(mcp): offload blocking filesystem IO in MCP config update (#3552)
* fix(mcp): offload blocking filesystem IO in MCP config update update_mcp_configuration resolved the extensions config path, probed its existence, read the raw JSON, wrote the merged config, and reloaded it — all blocking filesystem IO on the event loop (PUT /api/mcp/config). The whole read-modify-write after the async admin check has no interleaved awaits, so it moves into one _apply_mcp_config_update helper dispatched via asyncio.to_thread; the masked response is built on the loop. The secret-preserving merge, error codes, and the stdio command allowlist are unchanged. Found via `make detect-blocking-io`. Same class as #3457 / #3529 / #3551. Add tests/blocking_io/test_mcp_router.py anchor, verified red->green under the strict Blockbuster gate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): serialize concurrent config updates with a write lock Address review on #3552: offloading the read-modify-write to a worker thread dropped the implicit serialization the single-threaded event loop provided, so two concurrent PUT /api/mcp/config calls could interleave and clobber each other. Guard the offloaded RMW with a module-level asyncio.Lock to restore within-process atomicity (cross-process writers remain a separate, pre-existing concern). Add a serialization regression test (red->green: without the lock the tracked max concurrency exceeds 1). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: address mcp config blocking io review --------- Co-authored-by: ly-wang19 <ly-wang19@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
|
a0acdda103 |
fix(gateway): keep create_thread idempotent when the insert loses a race (#3800)
* fix(gateway): keep create_thread idempotent when the insert loses a race The /api/threads create endpoint checks for an existing row and then inserts, but the two steps are not atomic. When two requests create the same thread_id concurrently, one commits first and the other's INSERT fails on the duplicate primary key. The SQL-backed thread store raises, the handler catches it as a generic failure, and the loser gets a 500 — even though the docstring promises the call is idempotent. Re-read the row after a failed insert: if it is now present (the competing request won), return it instead of surfacing the conflict. A genuine write failure where the row is still absent keeps the 500. Covered by a regression test that simulates the lost race and asserts the endpoint returns the existing thread rather than erroring. Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com> * fix(gateway): scope create_thread race recovery and mirror owner reconciliation Address review on #3800: - Scope the insert-race recovery to sqlalchemy IntegrityError instead of a broad `except Exception`, so a non-race failure that coincides with an existing row is no longer silently returned as a 200. Any other error logs and surfaces as a 500. (The memory store overwrites on duplicate rather than raising, so it never reaches this path.) - Route both the fast path and the recovery path through a shared `_resolve_existing_thread` helper so the recovery performs the same trusted- owner reconciliation (claiming a legacy unscoped `user_id=None` row) the fast path does. Thread ownership no longer diverges based on which path resolved the record. Add regression tests for both: the recovery claims an unscoped row for a trusted owner, and a non-IntegrityError failure surfaces as a 500. --------- Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Tu NaichaoandGitHub
|
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
|
||
|
Daoyuan LiandGitHub
|
fa419b3e4c |
fix(goal): stop continuation_count double-bump in thread_changed_before_continuation stand-down (#4199)
_prepare_goal_continuation_input calls the same _persist closure twice with the identical next_count in one evaluation cycle: once to commit the real continuation, and again to record a thread_changed_before_continuation stand-down if a race is detected right after that commit. The second call re-passed continuation_count=next_count, so #4088's defensive max(continuation_count, current_count + 1) guard saw the first call's own write as a "current_count" bump and added another +1 on top of it - silently consuming 2 units of the continuation budget for a cycle that delivered zero actual continuations. #4088's guard is correct for the independent-concurrent-continuations race it targets; this is a separate call site incorrectly re-triggering that same guard against itself. The second _persist call no longer passes continuation_count, matching every other stand-down call site in this function - the count was already correctly committed by the first call. Adds a regression test mirroring the existing thread_changed_after_evaluation race test's checkpointer-wrapper technique, since this sibling branch had zero prior coverage. |
||
|
|
f0e56dde80 |
build(deps): bump h3 from 1.15.6 to 1.15.9 in /frontend (#4211)
Bumps [h3](https://github.com/h3js/h3) from 1.15.6 to 1.15.9. - [Release notes](https://github.com/h3js/h3/releases) - [Changelog](https://github.com/h3js/h3/blob/v1.15.9/CHANGELOG.md) - [Commits](https://github.com/h3js/h3/compare/v1.15.6...v1.15.9) --- updated-dependencies: - dependency-name: h3 dependency-version: 1.15.9 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
|
16919f7c52 |
fix(skills): reuse the resolved app config in the no-arg skills prompt section (#4160)
get_skills_prompt_section() without app_config resolved get_app_config() only to read container_path, then let the enabled-skills load fall back to the warm cache. On a cold start the cache is empty and the first call returns an empty skills list while the synchronously-loaded disabled section is populated, so manually assembled agents (create_deerflow_agent style integrations) got a prompt with no enabled skills. Rebind the resolved config so the storage and enabled-skills loads below use it too; when no config is resolvable the cache-only fallback is unchanged. Adds a cold-cache regression test. Fixes #4144 Co-authored-by: fancyboi999 <fancyboi999@users.noreply.github.com> |
||
|
yym36991andGitHub
|
e7e9c51988 |
Classify four HTTP identity sources (browser session, OIDC, IM channel (#4179)
binding, Internal Auth) and group IM bindings with direct HTTP under a shared platform-trust model. Explain Internal Auth trust boundaries: DeerFlow validates only the platform token, does not manage end-user accounts in users, and relies on the channel to maintain owner identity. Cross-link AUTH_DESIGN, API.md, and docs README |
||
|
Henry LiandGitHub
|
b68e1c686a | docs: add the link of LLM Space (#4195) | ||
|
Daoyuan LiandGitHub
|
2a7469cdbc |
fix(channels): dedupe GitHub webhook redeliveries (#4104)
* fix(channels): dedupe GitHub webhook redeliveries The inbound dedupe added for the IM channels in #3584 keys ChannelManager._is_duplicate_inbound on a top-level metadata["message_id"] plus a workspace id. The GitHub channel added later in #3754 never populated either: the X-GitHub-Delivery GUID was buried in metadata["github"]["delivery_id"] and no workspace id was set, so every GitHub delivery produced a None dedupe key and was never deduped. GitHub reuses the same X-GitHub-Delivery GUID when a delivery is retried after a timeout or replayed via the repo/App "Redeliver" button, so a redelivery re-ran the agent with real side effects (e.g. a duplicate PR comment) while the other channels absorbed an identical redelivery. fanout_event now stamps the dedupe identity the manager already consumes: - workspace_id = repo (globally unique, always present; mirrors Telegram/WeChat keying the workspace on the chat id) - metadata["message_id"] = f"{delivery_id}:{agent.name}" A single delivery fans out to N agents, so the id is scoped to (delivery, agent): an identical redelivery reproduces the same pairs (deduped) while two agents matching the same delivery keep distinct ids and both still fire. When the delivery header is absent the id is left None, so the manager fails open exactly as before. Tests: dispatcher-level coverage that the identity is stamped, stable across redelivery, and distinct per agent/delivery; a ChannelManager regression that an identical GitHub redelivery dispatches once while a new delivery and a second agent on the same delivery still fire. * fix(channels): scope GitHub dedupe id by owning user _inbound_dedupe_key indexes on (channel, workspace_id, chat_id, message_id). For GitHub, workspace_id and chat_id are both the repo, so the owning user was never represented anywhere in the key - fanout_event stamped the id as f"{delivery_id}:{agent.name}". Two different users each binding an agent of the same name (e.g. "reviewer") to the same repo+event therefore produced an identical dedupe id for both fan-out messages. ChannelManager._is_duplicate_inbound treated the second as a replay of the first and silently dropped it, even though GitHub delivered the webhook once and both users' agents legitimately matched. Fold match.user_id into the id: f"{delivery_id}:{match.user_id}:{agent.name}". Genuine redeliveries (same user, same agent, same delivery) still produce the same id and are deduped; two agents - same-named or not, same user or not - on one delivery now always keep distinct ids. Tests: new cross-user regression pins that two users' same-named agents both dispatch instead of the second being deduped against the first; the existing per-(delivery, agent) test and the literal id assertion in test_delivery_id_populates_inbound_dedupe_identity are updated for the new id shape. * fix(channels): point cross-user dedupe comment at its regression test Replace the inline reviewer-handle attribution with a pointer to test_dedupe_identity_distinguishes_same_agent_name_across_users, which ages better than a person's name in committed code. |
||
|
|
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
|
||
|
hataaandGitHub
|
1300c6d36b |
feat(authz): add pluggable AuthorizationProvider protocol and config scaffolding (Phase 0, #4063) (#4127)
Phase 0 of the RFC in #4063: scaffolding only, zero behavior change. New deerflow/authz/ package (sibling to deerflow/guardrails/): - AuthorizationProvider Protocol: authorize() + aauthorize() + filter_resources() - Principal/AuthzRequest/AuthzDecision/AuthzReason dataclasses - GuardrailAuthorizationAdapter: bridges AuthorizationProvider → GuardrailProvider so existing GuardrailMiddleware can enforce authz decisions without a new middleware New authorization config section (default enabled: false): - AuthorizationConfig wired into AppConfig alongside guardrails - Singleton load/reset mirrors GuardrailsConfig pattern - config.example.yaml documents the RBAC provider schema 29 tests covering protocol conformance, dataclass construction, adapter request/decision mapping, and config singleton behavior. Per RFC #4063 Phase 0 (foundations). Layer 1/2 wiring and Principal builder in services.py deferred to Phase 1. |
||
|
Zheng FengandGitHub
|
f3e4e5f8f0 |
ci(helm): publish chart to charts/ namespace prefix on GHCR (#4175)
* ci(helm): publish chart to charts/ namespace prefix on GHCR
Push the Helm chart to ghcr.io/<owner>/charts/deer-flow (via a `charts/`
prefix on the `helm push` target) instead of the bare `deer-flow`
package. This namespaces the chart apart from the image packages
(deer-flow-{backend,frontend,provisioner}) without renaming the chart:
Chart.yaml `name` stays `deer-flow`, so dir = chart name = in-cluster
resource/selector names, and no `nameOverride` hack is needed.
The chart is new in 2.1.0 (chart infra landed in #3987, after v2.0.0),
so 2.1.0 is the first chart release. Early nightly builds remain at the
legacy non-prefixed ghcr.io/<owner>/deer-flow.
Also refresh chart release docs:
- Replace the removed scripts/build-and-push.sh in the chart README with
raw `docker build`/`push` commands (contexts/args match container.yaml).
- Point NOTES.txt's empty-registry warning at the README section instead
of the removed script.
- Retarget RELEASING.md version examples from 2.2.0 to 2.1.0.
- Bump the chart README's helm prerequisite to 3+.
* docs(helm): require helm 3.8+ and note legacy chart package cleanup
Address PR #4175 review:
- bump documented minimum to helm 3.8 (OCI registry support stabilized
there; earlier 3.x needs HELM_EXPERIMENTAL_OCI=1)
- add a post-release note to delete/revoke the legacy bare
ghcr.io/<owner>/deer-flow chart package after 2.1.0
|
||
|
Daoyuan LiandGitHub
|
0dd90ccfde |
fix(agents): require config.yaml in update_agent's legacy-agent guard (#4166)
update_agent (the harness tool) and PUT /api/agents/{name} (the same
operation over HTTP) share an identical guard meant to block updates to
an agent that only exists in the legacy shared layout. The guard checked
bare directory existence:
if not agent_dir.exists() and paths.agent_dir(name).exists():
When memory is enabled, the first time a user chats with a legacy shared
agent, the memory writer creates a per-user directory containing only
memory.json (no config.yaml). agent_dir.exists() is then true, so the
guard never fires: the tool falls through to load_agent_config, which
resolves through to the legacy shared config via the already-hardened
resolve_agent_dir, and silently writes a brand-new config.yaml/SOUL.md
into the memory-only directory. That forks the agent for just this user;
every other user keeps reading the original shared config forever, with
no error or warning.
resolve_agent_dir itself was already hardened against exactly this
failure mode: it requires config.yaml to exist, not just the directory.
Mirror that condition at both call sites here.
|
||
|
AariandGitHub
|
8e96a6a252 |
fix(security): html-escape the conversation block in MEMORY_UPDATE_PROMPT (#4162)
format_conversation_for_update embeds raw user turns into the <conversation> slot of MEMORY_UPDATE_PROMPT. This is the most attacker-influenced input in the prompt, and it was unescaped: a message containing "</conversation><current_memory>..." closes the conversation block and forges a <current_memory> authority section for the extraction LLM, which can be steered into persisting an arbitrary high-confidence fact — and that fact is later injected into the lead-agent system prompt's <memory> block, which the prompt declares trusted. This is the last unguarded sibling of a rule the repo has established repeatedly. #4044/#4060 html-escaped the current_memory slot of this exact template; #4097 escaped the <memory> injection renderer. In updater.py the same .format() call escapes current_memory and leaves conversation raw. The memory updater sees raw text because InputSanitizationMiddleware only rewrites the ModelRequest and never mutates state, while MemoryMiddleware queues the raw state messages. Escape content with html.escape(quote=False), mirroring _escape_summary / _format_fact_line — after truncation so a trailing "..." cannot split an entity, on both human and assistant turns. Render-time only: no stored value is mutated, so the apply path is unaffected. The conversation function already strips <uploaded_files> here, so tag hygiene in this renderer is established. Scope is the memory updater. The summarizer's <new_messages> / <existing_summary> blocks are the same rule unguarded, but their output is quarantined as untrusted durable context rather than promoted to system authority; that hardening will be a separate change. |
||
|
Daoyuan LiandGitHub
|
656f6b364c |
fix(skills): recognize fully deleted public skill packages in review CI (#4169)
select_skill_packages() resolved every changed non-SKILL.md path to its owning package via an unconditional depth-3 fallback, then queued that path for review. When a PR deletes an entire public skill package (not just SKILL.md, but its scripts/assets/etc. too), the fallback still returned the package directory even though it no longer exists on disk post-deletion. The review CLI then reported a false structure.missing-skill-md blocker for a path that isn't there, failing CI on a routine, correct package removal. Skip a resolved package only when every changed file under it was a deletion and the package directory itself is gone from disk - i.e. the whole package was intentionally removed. A package left in a broken/partial state (e.g. SKILL.md deleted while sibling files remain) still resolves to an existing directory, so it is unaffected and continues to be queued and flagged. |
||
|
qin-chenghanandGitHub
|
713ee544b7 |
fix(agents): stop persisting base64 image data in checkpoint state (#4140)
* fix(agents): stop persisting base64 image data in checkpoint state (#4138) The viewed_images state field stored full base64-encoded image data, which was duplicated across every subsequent checkpoint (O(n * steps) growth). A single 1MB image viewed early in a conversation would be re-stored in every checkpoint for the rest of the session. Changes: - ViewedImageData: replace base64 field with lightweight metadata (mime_type, size, actual_path) - view_image_tool: store only metadata in state, no base64 encoding - ViewImageMiddleware: read image files from disk on-demand in before_model and encode base64 temporarily for the model call - Update all tests to use the new metadata-only format This is the first step of #4138. The base64 data is no longer in persistent state, but the injected HumanMessage (with base64 content) still appears in the checkpoint for the step where it was injected. Checkpoint retention policies and large tool result dedup are separate follow-up items. * fix(agents): address review feedback on #4140 - view_image_tool: remove stale 'convert to base64' comment, replace with 'validate contents'; drop redundant image_size reassignment and add a TOCTOU guard that rejects files changed between stat() and read(). - view_image_middleware: extract _read_image_as_data_url helper that re-checks size against the recorded value AND the absolute cap (_MAX_IMAGE_BYTES). Document the trust assumption for actual_path (server-set, not client-settable) in the helper docstring. - view_image_middleware: abefore_model now runs the blocking read+encode via asyncio.to_thread to avoid stalling the event loop on up to 20MB images. - tests: add coverage for OSError during read, file-changed-since-view (TOCTOU), and size-exceeds-cap branches. |
||
|
Daoyuan LiandGitHub
|
51bb19fa1b |
fix(channels): drop redundant GitHub review-comment webhook fan-out (#4131)
* fix(github): drop redundant pull_request_review_comment fan-out noise
GitHub fires one pull_request_review_comment webhook per inline comment
attached to a pull_request_review submission, in addition to the single
pull_request_review event for the review itself. A bot reviewer like
CodeRabbit commonly leaves 20-30 inline comments per review, flooding
the webhook with near-duplicate deliveries that carry nothing an agent
doesn't already have -- it fetches every inline comment itself via
`gh api` when it processes the parent pull_request_review event.
Filter these out in fanout_event() before the registry lookup / per-
agent loop, since the redundancy is a property of the event itself, not
of any specific agent binding. A companion comment is identified by
pull_request_review_id being set (it belongs to a review) and
in_reply_to_id being absent (it is not itself a reply within an
existing thread -- that case is a genuine new interaction and must
still fire).
* fix(channels): scope review-comment suppression to bindings that also see the review
The redundant pull_request_review_comment filter suppressed every
companion comment unconditionally, before the registry lookup even ran.
That premise only holds for a binding that also subscribes to
pull_request_review on the same repo -- events are opt-in per binding,
so a binding registered for pull_request_review_comment alone never
receives the parent review event and the companion comments were its
only delivery of the review's inline content. Suppressing those too was
a silent, total loss for that binding, not noise reduction.
Move the check into the per-agent loop so it only suppresses a matched
binding's companion comment when that same binding also has an active
pull_request_review trigger on this repo, reusing the existing registry
lookup rather than re-walking bindings by hand. Bindings subscribed to
pull_request_review_comment alone now always fire, matching the
opt-in-per-binding contract documented in triggers.py.
* fix(channels): close require_mention gap in review-comment redundancy gate
willem-bd's second review round found that the per-binding redundancy gate
(commit
|
||
|
qin-chenghanandGitHub
|
60e50537f3 |
fix(subagents): prohibit task tool in general-purpose system prompt (#4161)
* fix(subagents): prohibit task tool in general-purpose system prompt (#4159) The general-purpose subagent correctly lists `task` in disallowed_tools to prevent recursive nesting. However, the system prompt did not explicitly tell the LLM that `task` is unavailable. When the subagent sees the parent agent use `task`, it infers the tool is available and attempts to call it, triggering a LangGraph tool validation error. Add an explicit <tool_restrictions> block to the system prompt stating that `task` is NOT available and the subagent must NEVER attempt to call it. This prevents the LLM from attempting the call in the first place, rather than relying on runtime rejection. Add a regression test verifying the prompt contains the prohibition. * fix(security): register tool_restrictions in input sanitization denylist PR #4161 added <tool_restrictions> to general_purpose.py subagent prompt but did not register it in _BLOCKED_TAG_NAMES. The anti-drift test test_denylist_covers_framework_authority_blocks caught this: forging <tool_restrictions> in untrusted input could trick the model into believing it has (or lacks) tool restrictions it does not. Add 'tool_restrictions' to _BLOCKED_TAG_NAMES alongside the other subagent authority blocks (file_editing_workflow / guidelines / output_format / working_directory). |
||
|
|
9c77046d10 |
fix(middleware): drop orphan ToolMessages so strict providers don't 400 (#4080)
* fix(middleware): drop orphan ToolMessages with no matching AIMessage tool_call The rebuild loop only skipped ToolMessages whose tool_call_id matched a known AIMessage tool_call (to be re-emitted after it). An orphan ToolMessage whose tool_call_id has no matching AIMessage tool_calls fell through and was kept, leaving a dangling tool result that strict providers reject. Drop orphan ToolMessages as well, logging at debug. * fix(dangling): demote orphan-drop logs, add tool_call_id=None test - Update module/class docstrings to mention orphan ToolMessage handling - Accumulate orphan drop_count and emit a single logger.warning instead of per-message logger.debug calls - Simplify early-return logic: return None only when no patching AND no orphans were dropped - Add test_tool_call_id_none_orphan_is_dropped — a ToolMessage with tool_call_id=None is always an orphan and must be dropped Closes #4080 Co-Authored-By: Claude <noreply@anthropic.com> * fix(test): use model_construct for None tool_call_id test to bypass pydantic validation ToolMessage content='ghost' tool_call_id=None fails pydantic validation at construction. Use model_construct to simulate a corrupt/edge-case payload without tripping the string-only guard. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Daoyuan LiandGitHub
|
cdefd4a8aa |
fix(mcp): invalidate tools cache on config content + path, not just newer mtime (#4124)
* fix(mcp): invalidate tools cache on config content + path, not just newer mtime The MCP tools cache invalidated only on a strict extensions-config mtime `>` comparison and tracked no resolved config path, so `_is_cache_stale()` missed: - content changes with an unchanged mtime (same-second edits; object-store / network mounts that do not bump mtime); - content changes with a backward mtime (git checkout, cp -p / backup restore, tar / rsync preserving timestamps); - a resolved-path switch to a different config file with mtime <= the recorded value (structurally invisible — no path was tracked at all). On multi-worker (uvicorn/gunicorn) or stale-mtime deployments this leaves the LangGraph-embedded runtime and every non-writer worker serving stale MCP tools after `PUT /api/mcp/config`, breaking the module's documented promise that changes made through the Gateway API are reflected in the embedded runtime. Record the resolved config path and a `(mtime, size, sha256)` content signature at initialization and invalidate when the path OR the signature differs (`!=`), mirroring `config/app_config.py::get_app_config()` so the two runtime-editable config files share one content-based staleness signal. The per-call stat was already paid, and the small-file sha256 matches the cost app_config pays per request. The "config missing / not yet initialized" no-op behavior and the cache reset endpoint are preserved. Adds backend/tests/test_mcp_cache.py covering all three failure modes plus unchanged-file and forward-edit sanity cases. Also folds in an incidental backend/AGENTS.md doc-sync: the restart-required field list was missing `scheduler` and `run_ownership`, both present in reload_boundary.py::STARTUP_ONLY_FIELDS. * test(mcp): pin cache staleness contracts raised in review Two review observations on the content-signature cache fix, both raised as non-blocking design questions rather than bugs: - Whether relying on mtime+size alone (skipping the sha256) could ever be "optimized" back in, reopening the narrow same-second / identical-length swap gap the signature was built to close. - Whether the extensions config being deleted entirely after a successful init leaves the cache in a defined state, since current_signature flips to None and _is_cache_stale() returns False. Neither is a behavior change: both were already the intended contract, preserved verbatim from the pre-fix mtime-only code (which also returned False once the file could no longer be stat-ed). Record the reasoning inline and add regression tests that pin each contract so a future change cannot alter either silently: - test_same_mtime_same_size_swap_is_stale: a same-length server-name swap that leaves mtime AND size unchanged (the precise scenario from review, sharper than the existing same-mtime test, which also changes size) is still caught only because the sha256 is computed unconditionally. - test_config_deleted_after_init_is_not_stale: deleting the config file after init keeps the cache serving its last-known-good MCP tools instead of invalidating into an unconfigured state. Both new tests were confirmed to fail against a deliberately reintroduced version of the regression they guard (hash short-circuit / removed None-guard), then confirmed to pass against the real code. |
||
|
|
abd0c7a585 |
docs(api): document stateless /api/runs/stream endpoint (#4177)
* docs(api): document stateless /api/runs/stream endpoint
Record that clients can start a conversation without pre-creating a thread,
and that Gateway returns thread_id and run_id via the Content-Location header.
Co-authored-by: yym36991@gmail.com
* docs(api): show on_run_created for stateless stream continuation
The Python SDK example now captures thread_id/run_id via on_run_created
before documenting the follow-up call with config.configurable.thread_id,
matching the TS/cURL examples and langgraph-sdk 0.3.x behavior.
Co-authored-by: yym36991@gmail.com
|
||
|
AariandGitHub
|
c57cf221d3 | fix(security): html-escape the summary input blocks in the summarization prompt (#4182) | ||
|
|
e492bb1c68 |
fix(middleware): window LoopDetection tool-frequency counter so long runs don't false-trip (#4072)
* fix(loop-detection): decay per-tool frequency counter with a windowed deque The Layer 2 per-tool-type frequency guard in _track_and_check used a monotonic integer counter (freq[name] += 1) that never decayed or reset, so a long-running thread could trip the frequency warn/hard-stop even when calls were spread out over the whole run. Replace it with a deque of recent tool names trimmed to window_size, matching the windowed hash layer, and count occurrences within the window. Update _evict_if_needed and reset() to manage the new _tool_name_history storage. * address review: size Layer-2 freq window to the hard limit, not window_size The windowed freq_count is bounded by the deque length; reusing Layer-1's window_size (default 20) capped it below tool_freq_warn (30) / hard (50), making the Layer-2 guard dead code under the shipped default config. Size a dedicated _tool_freq_window = max(window_size, tool_freq_hard_limit, override hard limits) so a tight burst reaches the limit while spread-out calls still decay. Per @willem-bd review on #4072. Adds default-config regression tests: freq window >= hard limit, override coverage, and a tight-burst-with-distinct-args hard-stop under real defaults. Co-Authored-By: Claude <noreply@anthropic.com> * fix(#4072): docstrings describe windowed semantics; defaultdict+Counter for O(1) Addresses willem-bds three inline nits: 1. Docstrings for tool_freq_warn/tool_freq_hard_limit now explain the sliding-window semantics and reference _tool_freq_window sizing. 2. Hot-path deque() allocation avoided: _tool_name_history uses defaultdict(deque) instead of dict.setdefault(thread_id, deque()). 3. O(window) sum() scan replaced with mirrored collections.Counter (incremented on append, decremented on popleft) for O(1) freq_count. --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
|
88a1311fe8 |
fix(DB): legacy backfill creates missing Index objects on existing tables (#4090)
* fix(DB): legacy backfill creates missing Index objects on existing tables _run_baseline_create_all_sync calls create_all(tables=..., checkfirst=True). SQLAlchemy's Table.create(checkfirst=True) skips the table AND all its Index objects when the table already exists, so an index added to the ORM model after the table was first provisioned (e.g. the partial unique index uq_channel_connection_active_identity on channel_connections) is never created, and stamping 0001_baseline skips alembic's own create_index call too. Fix: after create_all, explicitly create every Index on every baseline table with Index.create(checkfirst=True), which checks for the specific index independently of the table. Co-Authored-By: Claude <noreply@anthropic.com> * docs(bootstrap): note future-index backfill collision risk for revisions Add a short forward-looking block under the legacy index-level backfill (at backend/packages/harness/deerflow/persistence/bootstrap.py:308-322) pointing the next contributor at the same shape risk the module already documents for _baseline_TABLE_NAMES: if a future post-baseline revision adds an index to a baseline table via op.create_index(...) without checkfirst=True, the backfill above will already have pre-created it and the upgrade will collide with 'index already exists'. Mirror safe_add_column and use checkfirst=True (or a future safe_create_index helper) in such a revision. * fix(#4090): scope legacy backfill index loop to _BASELINE_INDEX_NAMES willem-bd identified that the index loop iterated table.indexes (current ORM models full set), which includes post-baseline indexes like uq_runs_thread_active from 0004. Creating these prematurely before their owning revisions dedup step raises IntegrityError on legacy DBs with duplicate active rows -- bricking bootstrap. Changes: - Add _BASELINE_INDEX_NAMES: frozenset of 23 indexes that 0001_baseline actually creates (mirrors _BASELINE_TABLE_NAMES pattern) - Guard index-creation loop with _BASELINE_INDEX_NAMES filter - Wrap each idx.create() in try/except for graceful duplicate-data handling (uq_channel_connection_active_identity has no owning revision dedup) - Fix misleading checkfirst=True comment for alembic op.create_index - Add guard test pinning _BASELINE_INDEX_NAMES against 0001 output - Add regression test with duplicate active runs (the exact crash case) - Add regression test with duplicate channel connections (graceful handling) * fix(tests): update legacy backfill fixtures to current schema - Seed runs and channel_connections using the actual 0001 baseline columns. - Preserve the duplicate-active-row conditions exercised by the regression tests. - Apply ruff formatting required by backend CI. --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
|
13fd8e229a |
fix(checkpoint): persist run duration in checkpoints for history reads (#4118)
* fix: persist run duration in checkpoints for history reads * fix(checkpoint): harden run duration persistence * fix(checkpoint): persist run durations in metadata * fix(checkpoint): address review findings for run duration persistence - Add valid_duration_entry() shared validation helper (worker.py) - Rename _persist_run_durations -> persist_run_durations as public API - Import public persist_run_durations and valid_duration_entry in threads.py - Use BackgroundTasks for lazy backfill write to avoid blocking history reads - Add TODO about O(runs) growth of run_durations in checkpoint metadata - Document REGENERATE_HISTORY_RAW_SCAN_LIMIT doubling assumption * fix(checkpoint): replace pruning TODO with justification Accumulated run_durations overhead (~50 bytes/run_id) is negligible compared to messages channel blobs; no pruning strategy is needed. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
pclinandGitHub
|
fabadae416 |
add Volcengine Coding Plan to quikly setup (#4141)
* add Volcengine Coding Plan to quikly setup * modify model list |
||
|
Willem JiangandGitHub
|
192ea5155e |
Revert "fix(firecrawl): align tools with installed firecrawl-py v2 SDK surfac…" (#4165)
This reverts commit
|
||
|
|
b7b4b49bfe |
fix(firecrawl): align tools with installed firecrawl-py v2 SDK surface (#4151)
Replace FirecrawlApp with Firecrawl (v2 unified client) so the map/crawl/interact/extract tools target methods that actually exist: - map_url -> map (with sitemap kwarg instead of ignore_sitemap) - crawl_url -> crawl (keyword args instead of nested params dict) - interact -> scrape+actions (structured action dicts, not NL string) The installed firecrawl-py==4.23.0 has no map_url/crawl_url methods and interact(job_id, code=) does not accept url/actions params. All three tools previously deterministically returned Error:... before making a valid request. Also drop unused Optional import (ruff UP045). Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Huixin615andGitHub
|
24648194ae |
fix(frontend): show branch action only for completed turns (#4147)
* fix(frontend): gate branch action by completed turn * test(frontend): clarify branch turn boundaries |
||
|
AariandGitHub
|
41b137c4c4 |
fix(security): block forged framework tags in the input guardrail (#4155)
* fix(security): block forged framework tags in the input guardrail
InputSanitizationMiddleware's _BLOCKED_TAG_NAMES neutralizes forged
framework tags in untrusted input, but missed soul, thinking_style, and
critical_reminders -- which the lead-agent system prompt's System-Context
Confidentiality section names as internal framework data -- and the
underscore spelling system_reminder emitted by the todo/terminal
middlewares (only the hyphen spelling was blocked). A user, or an
attacker-controlled web_fetch/web_search page via the shared
neutralize_untrusted_tags primitive, could forge these blocks. Add them.
* fix(security): cover framework authority blocks as a class, not a subset
The confidentiality section declares every framework structured tag trusted
("and all other structured tags"), so the denylist must cover the authority
blocks as a class. Add the live blocks still passing both sanitization paths
(clarification_system, self_update, response_style, citations, skill_index,
available_skills, disabled_skills, memory_tool_system, durable_context_data,
slash_skill_activation), and pin the set against drift with a test that scans
the framework source and fails when a new block is not blocked.
* fix(security): scan the whole harness for framework blocks, fail closed
The drift guard added in the previous revision scanned a hand-listed set of
source files. That is the same forgot-to-update-a-list root cause the guard was
meant to eliminate, one level up, and it failed exactly that way: tool_search.py
was not in the list, so <mcp_routing_hints> and <available-deferred-tools> —
both rendered into the lead-agent system prompt via the {deferred_tools_section}
/ {mcp_routing_hints_section} placeholders — passed both sanitization paths
unneutralized.
Replace the file list with a repo-wide scan plus an exemption set that states a
reason per tag. The point is the failure direction, not the breadth: a new
framework block anywhere in the harness now turns CI red until it is either
blocked or exempted on the record, where before a block emitted from an unlisted
file was silently unguarded.
The scan reads raw source rather than AST string literals on purpose: an
attributed block built as an f-string splits its '>' into a separate literal
chunk, so an AST-on-literals scan misses it (verified against
<consolidation_candidates>). Raw source has one comment false positive, exempted.
Exempted with reasons: leaf/wrapper elements; the memory-updater and summarizer
prompts, which are built from checkpointed state rather than the ModelRequest
this middleware rewrites, so blocking them here would be false coverage, not
protection; and the MindIE provider wire format, parsed out of model output.
The scan surfaced five further live authority blocks beyond the two reported.
Subagents reuse _build_runtime_middlewares and therefore share this denylist, so
their system-prompt blocks are in the same class: file_editing_workflow,
guidelines, output_format, working_directory. goal_continuation is a
framework-authored hidden HumanMessage injected into the lead agent.
Also loosen the scanner regex to match the tolerance of _BLOCKED_TAG_PATTERN so
an attributed block cannot hide from the guard.
|
||
|
AnoobFengandGitHub
|
446fa03801 |
fix(context): resolve context compress bug (#4065)
* fix(runtime): persist original human input outside model sanitization * refactor(history): load thread messages by global event sequence * fix(frontend): make summarization rescue a transient history bridge * fix(frontend): old message not append tail 1. add identity anchor 2. add bridgeOrder * fix(frontend): lint error fix * fix: address review feedback and harden pagination coverage - defer transient history ref writes until after render commit - cover large middleware-only history scans - verify infinite-query refetch recalculates page cursors - document AI event types and anchor-weaving differences * fix: harden message pagination and enrichment - append unmatched live tails after canonical history - warn and stop when pagination has_more lacks a cursor - deep-copy restored UI messages to isolate model-facing content - log invalid event sequence and non-advancing cursor errors - pass user_id explicitly through event-store history queries - cover middleware-only AI runs across memory, JSONL, and DB stores * fix: address pagination review feedback * fix(frontend): checkpoint has unknow redener content, optimize the anchor policy * fix(frontend): unit test issue missed previously, remove the TanStack cache trimming * fix(gateway): harden message history queries and provenance - reject externally forged original_user_content metadata - validate provenance metadata in upload and sanitization middleware - make run lookups fail closed by default - batch feedback queries by run ID - align memory message filtering with persistent stores |
||
|
AariandGitHub
|
81b3ed0188 |
fix(skillscan): recognize remaining outbound network sinks (#4153)
_call_is_network_sink missed the HEAD/OPTIONS verbs on requests/httpx, socket.create_connection, and urllib.request.urlretrieve. A bulk env dump or reverse shell shipped through any of these slipped past the CRITICAL exfil/reverse-shell rules whenever the URL was assembled at runtime (the non-literal case the string-literal URL check can't cover). Also treat socket.create_connection as the socket primitive in the reverse-shell shape. http.client.HTTP(S)Connection is intentionally left out: only the lazy constructor is statically visible (the request()/connect() that performs the I/O is an instance method the call-name analyzer can't resolve), so flagging the constructor would hard-block benign code that only builds a connection object. Cover the alias-resolved forms too: the sink check runs on the name after from-import / import-as resolution, a path the suite exercised only on the env-read side (#4087) and not on the sink side. |
||
|
AariandGitHub
|
79cdd99fca |
fix(mcp): validate MCP tool names at load so deferred prompts stay inert (#4154)
* fix(tools): escape MCP tool names rendered into deferred prompts get_deferred_tools_prompt_section and get_mcp_routing_hints_prompt_section list MCP tool names into the <available-deferred-tools> and <mcp_routing_hints> system-prompt blocks without escaping, while the mirror get_skill_index_prompt_section (per its docstring) does escape. An MCP name is taken verbatim from an external server, so a crafted name could close the block and forge a framework tag. Escape names (and routing keywords) at render, mirroring the skill-index section. * fix(mcp): validate tool names at the load boundary Escaping at render only neutralizes < > &, so a tool name with newlines or markdown still injects free-form text into the deferred-tools prompt block. Deferred (tool_search) tools are never bound, so the provider's function-name check never runs on them. Drop any MCP tool whose name is not a valid identifier (^[A-Za-z0-9_-]+$) in get_mcp_tools() — the same charset the provider enforces at bind time — before it can enter the catalog or the prompt. Render-time html.escape stays as defense-in-depth. Mirrors the load-time skill-name validation in skills/storage/skill_storage.py. |
||
|
|
e2816eaa97 |
fix(models): scope the OpenAI-compat rules to BaseChatOpenAI, not a class-path allowlist (#4146)
* fix(models): scope the OpenAI-compat rules to BaseChatOpenAI, not a class-path allowlist * address review: drop redundant stream_usage helper, close test matrix - Remove _enable_stream_usage_by_default and its now-unused _OPENAI_COMPAT_USE_PATHS tuple. The class-field stream_usage fallback already sets stream_usage=True for every BaseChatOpenAI subclass (they all declare the field), so the helper's use-path allowlist gated nothing real — verified a no-op in prod, and the two stream_usage tests stay green on main with the helper present. Those tests used a BaseChatModel stub that does not declare the field; point them at a real ChatOpenAI capturing class so they exercise the fallback they now depend on. - Give the non-OpenAI normalization-skip test an actual api_base value so it exercises the skip path (api_base passed through verbatim, never rewritten to base_url). - Add an Unreleased CHANGELOG entry for the api_base behavior change on the five affected subclasses. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
|
4af6178358 |
feat(trace): add agent observability with Monocle (#4024)
* Add Monocle tracing Enable Monocle (OpenTelemetry tracing for LLM apps) with one setup call plus the monocle_apptrace dependency. setup_monocle_telemetry auto-instruments the frameworks already in use and writes traces to .monocle/. Additive; no changes to application logic. * Config-gate Monocle telemetry in the Gateway lifespan Addresses review on #4024: moves setup_monocle_telemetry out of agents/__init__ import time into the Gateway lifespan, gated by MonocleTracingConfig (MONOCLE_TRACING env, default off). Warns on the Langfuse/global-OTel-provider conflict and relies on monocle_apptrace's own duplicate-setup guard and existing-provider attach. Pins monocle_apptrace>=0.8.8 (+ uv.lock), adds .monocle/ to .gitignore, adds tests (default-off / toggle-on / no import-time setup), and documents exporters, Okahu, and the VS Code viewer in README, config.example.yaml, and backend/AGENTS.md. * Clarify Monocle/Langfuse single-provider guidance Make the docstring, warning, and AGENTS.md consistent with the README: only one library can own the global OpenTelemetry provider; Monocle initializes at startup before Langfuse's per-run handler, so enabling both drops Langfuse's spans — enable one OTel tracer (LangSmith, a callback, coexists fine). * Address review: optional extra, exporter validation, off-box warning, tests Responds to the second review round. - Make monocle_apptrace an optional extra (deerflow-harness[monocle], re-exposed as deer-flow[monocle]) following the boxlite/tui precedent, so a default install no longer pulls the OpenTelemetry stack. It stays pinned in the dev group for the tracing tests, and enabling MONOCLE_TRACING without the extra raises a clear install error. - Warn loudly at startup whenever any exporter other than `file` is configured, since those move prompts, tool inputs/outputs, and completions beyond the local .monocle/ directory. - Validate MONOCLE_EXPORTERS against the known exporter names and require OKAHU_API_KEY when okahu is selected, mirroring the Langfuse pattern. Validation runs from Monocle's own init (not validate_enabled) so a config typo can never fail agent runs; errors surface at Gateway startup instead. - Grow the tests from 5 to 13: caplog coverage for the Langfuse-conflict and off-box warnings, exporter validation cases, a stronger import-time regression that asserts the global TracerProvider is not replaced, and a subprocess double-invoke test exercising the real check_duplicate_setup. - Docs: config.example.yaml block retitled to a dedicated tracing header; README documents the [monocle] install and scopes tracing to Gateway runs. * docs: align Monocle README section with the other tracing providers Lead with what Monocle is and captures, drop the install step (the dev group already ships monocle_apptrace via uv sync; unusual installs get the RuntimeError), and point the missing-package error at the repo-native command (uv sync --extra monocle / deerflow-harness[monocle]). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Address review: verified Langfuse coexistence, lifespan test, scope docs Responds to the third review round. The Langfuse conflict claim was wrong, verified empirically against langfuse 4.5.1 in both init orders: whichever library initializes second reuses the existing global TracerProvider and attaches its own span processor, so neither side loses spans. Dropped the warning and its tests, corrected the README, AGENTS.md, and config.example.yaml statements, and pinned the verified behavior with test_coexists_with_langfuse (real monocle + real langfuse in a subprocess, no mocks). One honest caveat documented: both processors see all spans, so Monocle's exporters also capture Langfuse's spans when both are enabled. Also from the review: - Document the Gateway-only scope in AGENTS.md: the lifespan is the sole call site, so the embedded DeerFlowClient and TUI are not instrumented; embedded users call setup_monocle_tracing_if_enabled() themselves. - Add test_gateway_lifespan_initializes_monocle pinning the lifespan wiring. - Comment why MonocleTracingConfig.is_configured is intentionally coarser than LangSmith/Langfuse (composite validation lives in validate() at startup). - Note that monocle_exporters_list takes the comma-separated string as-is. - Module-level importorskip("monocle_apptrace") so minimal installs collect the test module cleanly. * docs: reword Monocle intro sentence * fix(tests): run the import-time regression in a subprocess test_no_import_time_setup deleted deerflow.agents* from sys.modules and re-imported to force __init__ to re-execute. The re-import creates new module objects, and restoring the old sys.modules entries afterwards leaves the parent package's attribute bindings pointing at the new ones, so any later test that resolves a deerflow.agents.* dotted path (monkeypatch.setattr in test_summarization_middleware, test_thread_data_middleware, and others) failed with "module 'deerflow.agents' has no attribute ...". Run the check in a subprocess instead: the import is genuinely fresh, the assertion is stronger (the provider must still be the SDK-less proxy, proving nothing was installed at any point), and no module identity leaks into the rest of the suite. * Address review: console warning scope, embedded hint, honest naming, doc alignment Responds to the post-approval review round: - Scope the off-box exporter warning to the remote exporters (okahu, s3, blob, gcs): console writes to local stdout and no longer trips it. config.example.yaml's data-handling note now distinguishes file / console / remote likewise. - Rename MonocleTracingConfig.is_configured to is_enabled so the boolean reads as what it checks; the exporter-dependent credential check stays in validate(), run at Gateway startup. - Hint on the embedded path: build_tracing_callbacks() logs a debug line when MONOCLE_TRACING is set but setup never ran in this process, so embedded DeerFlowClient/TUI users are not left with silent no-op tracing. Backed by a process-global setup flag. - Re-export setup_monocle_tracing_if_enabled from deerflow.tracing, matching the package convention. - Note the deliberate fail-open-at-startup contrast with LangSmith/Langfuse in the lifespan, and the OTel SDK-internals dependency in the coexistence test. - Test hygiene: clear MONOCLE_* env in the tracing config/factory fixtures; reset the setup flag in the monocle test fixture; reword the README Langfuse-spans claim as the shared-provider inference it is. - Document that .monocle/ trace files are never rotated or cleaned up. * fix(tests): pin the factory logger level in the embedded-hint tests configure_logging() from earlier tests in the full suite pins an explicit INFO level on the logger hierarchy, so a root-level caplog.at_level(DEBUG) never sees the factory's debug hint. Scope caplog to deerflow.tracing.factory so the test is independent of suite ordering. * Address review: co-export disclosure, lifespan failure test, exporter parse dedup - Off-box warning now notes that Langfuse's spans are exported too when both providers are enabled and share the global OTel provider; pinned both ways by tests. - Pin the lifespan fail-open contract: a raising Monocle setup is logged and the Gateway keeps serving (pragma dropped now that the path is exercised). README notes a config error is reported at startup and tracing stays off until restart. - Hoist exporter parsing into MonocleTracingConfig.exporter_list so validate() and the off-box warning cannot diverge, and note the upstream coupling on the exporter allow-list. - Reduce config.example.yaml's Monocle block to a pointer; the capture, retention, and data-handling detail lives in README's Monocle section. --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Yufeng HeandGitHub
|
e361122b9a |
fix(security): html-escape subagent descriptions before the <subagent_system> block (#4157)
A custom subagent's description is agent-editable (persisted by setup_agent / update_agent) and is rendered into the <subagent_system> block of the lead-agent system prompt via the available-subagents listing. It was interpolated raw, so a first line like "</subagent_system><system-reminder>..." could close the block and forge a framework-reserved tag inside the system-role prompt. Escape it with html.escape at the render site, matching the sibling fixes for <soul> (#4137), memory facts (#4097), skill metadata (#4128), and remote content (#4099/#4002). Built-in descriptions are trusted constants and stay untouched. Adds a red/green regression test mirroring test_soul_prompt_injection.py. |
||
|
|
8cc4b3abeb |
fix(sandbox): the sandbox provisioner api exposes endpoints need to check API KEY (#4116)
* fix: V-001 security vulnerability Automated security fix generated by OrbisAI Security * fix(sandbox): wire provisioner API key through backend client and config RemoteSandboxBackend now accepts an api_key parameter and sends it as X-API-Key on all five provisioner HTTP calls (list, create, destroy, is_alive, discover). AioSandboxProvider reads provisioner_api_key from SandboxConfig and forwards it at construction time. SandboxConfig formally declares the field; config.example.yaml documents it under Option 4; docker-compose-dev.yaml threads PROVISIONER_API_KEY into both the provisioner and gateway containers so a single .env entry covers both sides. Tests: monkeypatch PROVISIONER_API_KEY and send X-API-Key headers in the five parametrized threading tests (previously 401-failing); new test_auth_middleware asserts /health is open, /api/* rejects no-header and wrong-key with 401, and accepts the correct key. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(provisioner): correct fail-closed auth docs and fix test mock signatures The first round of PR #4116 fixes left four blocking issues: - Mock signatures in test_remote_sandbox_backend.py (17) and test_aio_sandbox_provider.py (2) didn't accept the new headers= kwarg, causing TypeError on every provisioner HTTP call test. - sandbox_config.py and config.example.yaml described the auth as optional ("leave unset to disable") but the middleware is fail-closed: an unset PROVISIONER_API_KEY causes 401 on every /api/* request. - .env.example had no PROVISIONER_API_KEY entry, leaving users with an empty value and silent 401s. - No test covered the PROVISIONER_API_KEY="" fail-closed path. Fix all four: add headers=None to all mock signatures, correct the field description and example comment to state that both sides must have the same key set, add PROVISIONER_API_KEY to .env.example with generation guidance, add test_auth_middleware_unset_key, and add a logger.warning on auth rejection for observability. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> |
||
|
AariandGitHub
|
e32456d337 |
fix(memory): coerce stored confidence in the three remaining raw reads (#4034)
* fix(memory): coerce stored confidence in the three remaining raw reads `_coerce_source_confidence` exists because `memory.json` is user-editable and written across versions: a `confidence` that is null, a str, a bool, out of range, or non-finite still has to read back as a usable score. Consolidation already routes every stored read through it, and #4023 did the same for the max_facts trim. Three reads still take the field raw. `_build_staleness_section` formats it with `f"{conf:.2f}"` — a str raises ValueError, None raises TypeError. `_do_update_memory_sync`'s `except Exception` swallows that into `return False`, aborting the whole memory-update cycle permanently, since the offending fact is then never rewritten. The staleness cap's `sort(key=lambda f: f.get("confidence", 0))` ranks the facts it is about to delete. A str raises the same way; the values that don't raise mis-rank instead. `true`/`inf` outrank a genuine 0.9 and push it into the removal slot; an absent key ranks 0 and is deleted first; `nan` compares false against everything, so which fact dies depends on where the corrupted one happens to sit in the file. `search_memory_facts` — the `memory_search` tool, added in #4023 — ranks results the same raw way and then truncates to `limit`, so the same mis-ranking hands the model the wrong facts, or fails the tool call outright on a str. All three now read through the helper, so an unusable stored confidence ranks as unknown (0.5): neither kept ahead of a real score nor evicted before one. The two sorts run in opposite directions, and 0.5 is load-bearing in both. * test(memory): anchor the confidence delta at every coerced read The str-based tests raise on main, so they cannot go red for any stored confidence that mis-ranks without raising. `true`/`false`/`inf` and an absent key never reached the except handler at all: bool subclasses int, so `true` ranked 1.0; `inf` outranked every real score; an absent key ranked 0. Silent fact loss in the staleness cap, wrong results out of `memory_search` — no log line either way. Parametrize both ranking sorts over the four non-raising inputs that fall to the 0.5 default, each pitted against a genuine neighbour chosen so the survivor flips. The sorts run in opposite directions, so the one matrix covers eviction from both ends. `nan` is excluded from those: its old rank is undefined rather than pinned to an end of the order, so one fixed input order happens to yield the correct survivor. Its real property is order-independence, asserted across both fact orders. The staleness prompt formatter is pinned by rendered value, not merely by not raising: 1.5 → 1.00, -0.3 → 0.00, and inf/nan/true/None → 0.50, matching the consolidation prompt that reads the same field through the same helper. |
||
|
|
b53c1ae0e0 |
fix(runs): cancel degrades to lease takeover for multi-worker (#4064)
* fix(runs): cancel degrades to lease takeover for multi-worker Work item 4 of the multi-worker ownership epic (https://github.com/bytedance/deer-flow/issues/3948). Problem: POST /runs/{run_id}/cancel landing on a non-owning worker returns 409 — the cancel button silently fails under GATEWAY_WORKERS>1 with no sticky routing. cancel() required the current worker to hold the in-memory task/abort_event, which any non-owner pod cannot satisfy. Changes: - RunManager.cancel() returns CancelOutcome enum (cancelled / taken_over / lease_valid_elsewhere / not_active_locally / not_cancellable / unknown) instead of bool, so the router can map each outcome to the right HTTP response. - New store primitive claim_for_takeover(): a single atomic conditional UPDATE that marks a run as error only when status IN (pending, running) AND (lease IS NULL OR lease < now - grace). Closes the stale-read / concurrent-heartbeat race — if the owner renews between our read and write, the UPDATE matches 0 rows and we surface lease_valid_elsewhere. - HTTP cancel + stream-join endpoints route on CancelOutcome: cancelled -> 202 (or 204 with wait=true); taken_over -> 202 immediately (no SSE streaming — the run is terminal on another worker, streaming would hang); lease_valid_elsewhere -> 409 + Retry-After header computed from lease_expires_at + grace_seconds. - RunManager.grace_seconds exposed as a public property; the router no longer reaches into _run_ownership_config. - _is_lease_expired extracted to a module-level function, shared by RunManager.cancel() and MemoryRunStore.claim_for_takeover(). - GATEWAY_WORKERS=1 + heartbeat_enabled=false is zero-regression: the non-local path short-circuits to not_active_locally, preserving the original 409 behaviour the existing tests pin. Tests: 12 new (5 store primitive + 4 cancel-takeover unit + 3 HTTP including a regression guard verifying POST /stream?action=interrupt on a dead-owner run returns 202 instead of hanging on SSE). 244 directly-related tests pass; 36/36 blocking-IO gate pass. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(runs): guard update_status and self-terminate on takeover Two defenses close a split-brain window where the original owner could overwrite a peer's takeover status: - update_status (SQL + memory store) now guards on status IN ('pending','running'). When takeover already set the row to 'error', the owner's final status write matches 0 rows and is dropped. - _persist_status: when update_status returns False, check whether the row exists before attempting recovery via put(). If the row exists (takeover by another worker), skip recovery instead of blindly upserting over the takeover. - Heartbeat _renew_leases: when update_lease returns False (row no longer pending/running or owner changed), cancel the local task so wasted CPU is bounded to the next heartbeat tick (~10s) instead of the full task lifetime. Also fix three reviewer feedback items: - Re-fetch the store row when cancel() returns lease_valid_elsewhere, so Retry-After uses the owner's freshly-renewed lease instead of a stale value from request start. - Fallback 'unknown' in takeover error message when owner_worker_id is NULL (pre-ownership data). - Remove dead else-10 branch from grace_seconds property (unreachable — all callers are downstream of the heartbeat_enabled guard). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * test(runs): pin split-brain defences from update_status guard + heartbeat Three tests lock down the takeover authoritativeness so a late-running owner cannot overwrite a peer's claim: - update_status must reject writes when the store row is already terminal (taken over by another worker). - _persist_status must skip row-recovery via put() when the row exists but has been taken over. - Heartbeat _renew_leases must cancel the local task when update_lease returns False (row claimed by another worker). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(runs): precise outcome + log when local cancel loses to peer takeover Two reviewer precision nits on the split-brain defence: - _persist_status: branch the skip-reason log on existing["status"]. error → WARNING "peer takeover" (anomalous); interrupted/success → INFO "local cancel/completion race" (expected when user hits stop as the run finishes). Stops noisy false-positive takeover warnings in operator logs. - cancel() local path: when _persist_status returns False, re-check the store. If a peer's claim_for_takeover flipped the row to error between our in-memory cancel and the guarded update_status, surface taken_over instead of cancelled so the client sees a status consistent with the store. Test: test_cancel_returns_taken_over_when_peer_claims_during_local_cancel pins the race outcome. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(runs): widen update_status guard, de-duplicate lease helpers, add coverage Round 3 of reviewer feedback: - Widen update_status guard to status IN ('pending','running','interrupted'). The original guard blocked interrupted→error (the rollback finalize path), losing the "Rolled back by user" message. interrupted is now permitted while error/success stay locked — takeover protection unchanged. - claim_for_takeover False now re-reads the store row to distinguish causes: owner renewed lease → lease_valid_elsewhere; row went terminal → not_cancellable; another worker already took it over → taken_over. - Extract _raise_lease_valid_elsewhere() helper to de-duplicate the 409+Retry-After block shared across cancel_run and stream_existing_run. - Extract _lease_expired_or_null() in persistence/run/sql.py to de-duplicate the lease-expiry SQL WHERE clause shared by claim_for_takeover and list_inflight_with_expired_lease. - 11 new tests: 5 SQL-layer claim_for_takeover (expired/valid/NULL/ terminal/nonexistent), 3 _compute_retry_after unit (NULL/unparseable/ normal), 2 claim re-read precision (terminal/takeover), 1 stream endpoint 409+Retry-After. Not addressed (non-blocking, reviewer agreed): - The 2–3 store.gets in the takeover cold path: optimizing the API to accept a pre-fetched record would couple the router to the manager more tightly than justified by the perf gain. - The lease-expiry inline loop in MemoryRunStore.list_inflight_with_- expired_lease pre-computes cutoff once for all rows; switching to the shared _is_lease_expired helper would recompute datetime.now() per row with no real benefit. 260 related tests pass; 36/36 blocking-IO gate pass; ruff clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(runs): de-duplicate lease-expiry helper, restore defensive fallback Address final round of review feedback: - Extract is_lease_expired to deerflow.utils.time (no _ prefix, public utility). Manager and MemoryRunStore now import from the same place instead of the store reaching backward into the manager for a private function. - Restore defensive else-10 fallback in grace_seconds property (removed in an earlier round). The guard is unreachable for current callers but protects future ones from AttributeError. - Comment the transient in-memory interrupted vs store error state when a local cancel is superseded by a peer takeover. - Comment the max(1, ...) floor in _compute_retry_after — the floor is a lower bound, not a poll interval; clients should apply jitter. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: rayhpeng <rayhpeng@gmail.com> |
||
|
Daoyuan LiandGitHub
|
3e7baba39a |
fix(models): apply stream_chunk_timeout default to all BaseChatOpenAI subclasses (#4102)
* fix(models): apply stream_chunk_timeout default to all BaseChatOpenAI subclasses The 240s stream_chunk_timeout default (issue #3189, PR #3195) was scoped to a class-path allowlist of only ChatOpenAI and PatchedChatOpenAI. Every other OpenAI-compatible provider that subclasses BaseChatOpenAI — VllmChatModel, MindIEChatModel, PatchedChatDeepSeek, PatchedChatMiMo, PatchedChatStepFun and PatchedChatMiniMax — was excluded, so they kept langchain-openai's aggressive 120s built-in chunk-gap timeout and, worse, silently discarded a user's explicit stream_chunk_timeout override from config.yaml. Issue #3189 was itself reported on mimo-v2.5 (PatchedChatMiMo), the exact class the original fix left out. Gate the injection on issubclass(model_class, BaseChatOpenAI) instead of the string allowlist, so any OpenAI-compatible subclass inherits the default and honors an explicit override. Genuinely non-OpenAI clients (e.g. ChatAnthropic) stay excluded and still have the kwarg dropped before it reaches a constructor that would divert it into model_kwargs and fail at request time. * fix(models): address review nits on stream_chunk_timeout default Correct the module-level comment above _DEFAULT_STREAM_CHUNK_TIMEOUT_SECONDS: langchain-openai's built-in stream_chunk_timeout default is 120s, not 60s (BaseChatOpenAI.stream_chunk_timeout's default_factory reads LANGCHAIN_OPENAI_STREAM_CHUNK_TIMEOUT_S with a 120.0 fallback). Simplify the BaseChatOpenAI gate in _apply_stream_chunk_timeout_default from `isinstance(model_class, type) and issubclass(model_class, BaseChatOpenAI)` to just `issubclass(...)`. The sole caller passes model_class from resolve_class(), which already raises before returning anything that isn't a type, so the isinstance half can never be False there. Also soften the docstring's non-OpenAI-client bullet: ChatAnthropic declares extra="ignore" and silently drops an unrecognized kwarg rather than diverting it into model_kwargs and failing at request time (that failure mode is specific to other OpenAI-style clients). |