mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-08-10 14:58:46 +00:00
86 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
a8bf54cbbb
|
feat(skillscan): detect exfil through instance/dataflow network clients (#4265)
* feat(skillscan): detect exfil through instance/dataflow network clients * fix(skillscan): resolve client handles with Python lexical scopes _walk_client_nested_scope copied every live handle into a nested scope after excluding only parameters. That is not Python's name resolution: a class namespace is not a closure scope for its methods, a function-local binding shadows an enclosing name across the whole body, and comprehensions bind their targets in a scope of their own. Benign skills were therefore blocked as python-env-dump-exfil (CRITICAL) even where the outbound-looking call provably cannot run on the tracked client. Derive each scope's bindings lexically instead: - a class body reads its enclosing scope, but the names it binds are not passed into the methods defined in it; - comprehensions are their own scope, so only the outermost iterable is evaluated outside and every for-target shadows first; - a function-local prepass excludes names the body binds anywhere, with global/nonlocal opting back out. Detection is unchanged where the client really is reachable: a comprehension calling an unshadowed handle, a method closing over an enclosing function's handle, and a global-declared module handle all still block. * fix(skillscan): apply target rebinding in Python evaluation order The generic ast.iter_child_nodes() fallback yields fields in declaration order, which for `for`/`async for`, `:=` and `+=` puts the binding target ahead of the expression Python evaluates to produce it. Visiting the target first dropped the handle before the call that actually runs on it, so `for s in s.post(host, ...)` reported nothing even though Python calls post() on the client before binding the loop target. Match captures bind through a plain `name` string rather than a Name node, so the Store branch never saw them and a rebound name kept a stale handle. Assignment expressions inside a comprehension were applied only to the comprehension-local map, leaving the containing scope stale as well. Give every bind-after-evaluate construct its own branch: - `for`/`async for` walk the iterable against the pre-loop binding, then rebind the target, then walk the body; - `:=` walks its value first and binds into the containing scope too, since PEP 572 puts a comprehension's walrus target there; - `+=` walks its value before dropping the target; - match captures drop the handle, matching how a rebind under `if`/`try` that may equally not execute is already treated. The bypasses this closes are the reason detection widens here; the comprehension walrus and match cases narrow it back where the client provably cannot be the receiver. * fix(skillscan): preserve scoped client handle semantics * fix(skillscan): scan sinks inside assignment target expressions The Assign/AnnAssign, AugAssign, For/AsyncFor, with, and comprehension branches evaluated only the value and rebound the target, so a client call placed in an attribute receiver or a subscript value/index -- both evaluated at bind time -- was never scanned. os.environ could exfil through a tracked client while python-env-dump-exfil reported nothing. Add _walk_client_target_exprs to walk the executable parts of a binding target (attribute receiver, subscript value+slice, recursing through tuple/list/starred) in Python evaluation order, without treating Store name leaves as reads. The AnnAssign annotation expression is walked too. Name-leaf invalidation is unchanged. * fix(skillscan): apply assignment targets and annotations in runtime order The round of assignment-target scanning walked every target then rebound the names in one batch, and always walked an AnnAssign annotation before the target. Python instead binds chained and destructured targets left to right (so `session = out[session.post(...)] = cfg` runs the subscript on the already-rebound name), evaluates a variable annotation only in module or class scope (never in a function, never under `from __future__ import annotations`), and evaluates an executed annotation after the target. The scanner therefore hard-blocked benign skills. Bind each target left to right via _bind_client_targets (walk its executable sub-expressions against the current bindings, then rebind before the next target), and walk an annotation only for the nodes _evaluated_annotation_nodes marks as actually evaluated, after the target. Target-expression scanning and name-leaf invalidation are otherwise unchanged. * fix(skillscan): scan client sinks in evaluated function-signature annotations A function's parameter and return annotations are evaluated at def time in the enclosing scope, like its decorators and defaults, so a tracked client sink placed in one is a real egress. _client_scope_prelude walked the decorators and defaults but not the annotations, so os.environ could exfil through `def f() -> session.post(host, json=dict(os.environ))` while python-env-dump-exfil reported nothing. Record function/async-function defs in _evaluated_annotation_nodes (their signatures evaluate at def time unless from __future__ import annotations postpones them) and, for those nodes, add the parameter and return annotation expressions to the enclosing-scope prelude walk. * fix(skillscan): match runtime evaluation order for annotations and except handlers * fix(skillscan): scope try/match branches to their own selection state * fix(skillscan): propagate branch bindings to everything that observes them A branch's net effect has to be visible exactly where Python makes it visible. The walker isolated `except`/`else`/`match` bodies into scope copies and then discarded them, so a client created on the branch was invisible to `finally`, to the code after the statement, and to anything defined inside the branch, while a name the branch replaced stayed a sink receiver. - `except*` clauses are sequential, not alternatives: thread one scope through body, clause types and clause bodies in source order instead of reusing the mutually exclusive copies ordinary `except` needs. - Fold each `except`/`else`/`match` branch's net effect back into the scope that `finally` and the following code read, and make the branch scope what nested definitions close over. - Keep a fallthrough scope across `match` guards, so a guard that returned false still hands its side effects to the next case, while pattern captures stay isolated to their own case. - Keep an `as` target path-local: Python unbinds it only on the path that ran, so dropping it for every path erased a live handle where no such handler executed. Adds a 14-case runtime-oracle regression covering both directions at every branch site; each of the ten guards was deleted on its own to confirm the test that pins it goes red. * fix(skillscan): join alternative branches instead of overwriting one with another Only one of a statement's alternative branches runs, but the walker folded each one into a single destructive binding map. Whichever branch was visited last therefore decided the state: a handler that rebinds the name erased a sibling that leaves the client in place (missing a client Python really calls), and a handler that builds one was credited on paths where it never ran (inventing a CRITICAL sink). Alias targets had the mirror problem, since only key presence was compared, so replacing `import x as name` on a live branch was ignored. - Join alternatives into a may-state: a name stays a sink receiver when any feasible branch leaves it a client, and stops being one only when every feasible branch replaced it. Alias targets join toward the target that can still name a constructor. - Treat each `except*` clause as optional rather than threading every clause body unconditionally, so a clause whose type never matched cannot erase a handle the next clause calls. - Keep the fall-through state (no exception raised, no case matched) as one more alternative, and drop it only where the source decides the outcome: a literal always-raising body selecting one handler, a literal exception group choosing `except*` clauses, a wildcard or literal-equal `match` case. Also corrects an existing match-capture test that asserted the non-exhaustive case is benign: the runtime oracle shows the original client still takes the call on the path where nothing matched. Adds runtime-oracle regressions for both directions at every alternative site; each of the nine guards was deleted on its own to confirm the test pinning it goes red. * fix(skillscan): model feasible conditional client flow * fix(skillscan): preserve feasible control-flow outcomes * fix(skillscan): model expression evaluation paths * fix(skillscan): narrow instance-client detection to lexical statement order The construction-to-use signal had grown into a path-aware interpreter: exception selection, except* subgroup consumption, match capture timing, finally override, comprehension laziness, annotation evaluation order, and may-state joins over feasible branches. That is the heavyweight analysis RFC #2634 rules out of Phase 5, and because the signal feeds a CRITICAL rule it hard-blocks skill installation, so every ambiguity it resolved by over-reporting cost a benign skill instead of a human review. Replace it with ordinary statement order over a one-level handle map: a known constructor bound to a simple name (including `with ... as`), a direct outbound method call on that name in the same lexical scope, rebinding invalidation, and name-to-name alias propagation so `s = session` does not shed the handle. A sink is recorded at the call, so a rebind after the call cannot retract it. Compound statements are not interpreted. Every name an if/try/except*/loop/ match may bind is dropped before its bodies are walked, and each body is walked from an isolated copy. Dropping before rather than after is what keeps a `finally` that runs after a handler rebound the name, a later except* clause, and a second loop iteration from reporting a client the runtime never calls. Bodies are still walked, or wrapping any construction in `if True:` would be a universal bypass. Lexical scoping is unchanged: class namespaces are not closures for their methods, comprehension targets and function-local bindings shadow, and alias visibility stays per scope. The cases this gives up are false negatives by construction and are recorded in #4296, pinned by a test that asserts the runtime really calls the client while the scanner stays silent. Verified: 102 SkillScan tests; full suite 8 failed / 7851 passed, the same network-dependent web-fetch tests that fail on clean main; per-clause red check 12/12 guards red; 925 repo-owned files scanned branch vs main with 0 new and 0 lost CRITICAL findings; #4158 bypass BLOCKED and #4153 false positive allowed with 0 findings through the real enforce_static_scan gate. * test(skillscan): pin closure boundary * refactor(skillscan): narrow client handle analysis * fix(skillscan): close client handle correctness gaps * fix(skillscan): require proven client imports |
||
|
|
3ed2e1f1d9
|
fix(config): close out #4124 review follow-ups (shared signature helper, resolve_config_path None contract) (#4275)
* fix(config): close out #4124 review follow-ups Extracts the (mtime, size, sha256) content-signature helper that was duplicated between config/app_config.py and mcp/cache.py into a new config/file_signature.py, and fixes ExtensionsConfig.resolve_config_path() to return None instead of raising FileNotFoundError when an explicit config_path argument or DEER_FLOW_EXTENSIONS_CONFIG_PATH points at a file that has since been deleted -- the exact resolution mode Docker dev/prod uses per AGENTS.md, so the MCP tools-cache staleness check could raise instead of degrading to "not stale". Both were flagged by willem-bd in review on #4124 and explicitly deferred there ("flagging for visibility", "leaving the extraction as the follow-up you suggested"). * fix(config): keep explicit extensions-config paths fail-loud resolve_config_path() previously turned every missing-file case into a clean None, including an explicit config_path argument or DEER_FLOW_EXTENSIONS_CONFIG_PATH (the exact mode Docker dev/prod uses). That silently downgrades a bad Docker mount, typo, or deleted production config to "no extensions" instead of surfacing the misconfiguration, per fancyboi999's review [P1] and willem-bd's follow-up notes on this PR. Restores FileNotFoundError for the two explicit modes (config_path argument, DEER_FLOW_EXTENSIONS_CONFIG_PATH); only the fallback search mode (no explicit path/env var, nothing found in the usual locations) still returns None, since that is the legitimate "extensions were never configured" case. The one caller that needs the old fail-soft behavior -- the MCP tools-cache staleness check, which re-resolves the path on every get_cached_mcp_tools() call -- gets a narrow, local catch instead (deerflow.mcp.cache._resolve_config_path) so a config file going missing mid-run still degrades the cache to "not stale" rather than crashing a hot per-request path. Also hoists the double os.getenv() read in the env-var branch into a local, per willem-bd's nit. Adds resolver-level tests for both explicit-path and env-var raises, a dedicated search-mode None regression test, and updates the existing MCP-cache docstrings to describe the corrected split. |
||
|
|
5b65d543b1
|
fix(channels): key inbound dedupe on chat-scoped workspaces (#4287)
* fix(channels): key inbound dedupe on chat-scoped workspaces * fix(channels): release the inbound dedupe key on swallowed transient failures _release_inbound_dedupe_key runs from _handle_message's generic exception handler, but three sites handle their error in place and never re-raise, so the key recorded on receipt survives the full dedupe TTL and the provider's redelivery -- the retry that would have recovered the failure -- is dropped: - _handle_streaming_chat swallows every stream error into its finally block - the fire-and-forget runs.create busy branch - the non-streaming runs.wait busy branch Keying chat-scoped workspaces gives telegram/feishu/wechat/dingtalk a dedupe key for the first time, which newly exposes them to this. The mechanism is pre-existing, not introduced here: WeCom already had a key (streaming plus aibotid) and shows the same black hole on main. Also parametrize the dispatch dedupe test over all three chat-scoped providers so the streaming path is covered end-to-end, and restate the workspace-fallback comment in terms of what the chain actually guarantees -- both fallbacks are appended last and gated on every earlier source being absent, so they can only turn "no key" into a key, never change one. * fix(channels): publish final reply before dedupe release --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
|
0f088033fe
|
fix(gateway): prefer X-Trace-Id over metadata.deerflow_trace_id when header is set (#4283)
When TraceMiddleware binds a valid inbound X-Trace-Id, worker resolution prefers that id over config.metadata.deerflow_trace_id so logs, response headers, Langfuse, and runtime context stay aligned. Also add trace-context marker reset coverage for the inbound-header flag. |
||
|
|
a028dfd5fb
|
feat(auth): add keep me signed in login option (#4255)
* feat(auth): add keep me signed in login option * fix(auth): address remember-login review feedback |
||
|
|
f8bef42a04
|
fix(wechat): validate timing configuration (#4280) | ||
|
|
a0e1d82ef4
|
fix(workspace-changes): offload blocking filesystem IO in text-cache lifecycle (#4268)
* fix(workspace-changes): offload blocking filesystem IO in text-cache lifecycle capture_workspace_snapshot and record_workspace_changes offload their scans via asyncio.to_thread, but ran the snapshot text cache's whole lifecycle on the event loop: roots resolution (os.path.abspath), tempfile.mkdtemp, and shutil.rmtree on both the capture-failure branch and record_workspace_changes' finally. That finally runs on every agent run, including abort paths, so each run removed up to max_files cached texts on the loop. Offload the roots + mkdtemp prep through one _prepare_capture worker hop, and route both rmtree call sites through _remove_text_cache_dir. Cleanup stays best-effort: it swallows and logs, so a failing cleanup cannot replace the exception or result already in flight. asyncio.shield is deliberately not used -- to_thread submits to the pool immediately, so cancelling the future does not stop the running thread and the cache is still removed under single and repeated cancellation. Externally observable behavior is unchanged. Found via `make detect-blocking-io`: these were the last 2 HIGH findings in the repo, which now reports none. The roots resolution is invisible to that scanner (sync helper, cross-file call) but blocks the same async path, and the anchor cannot reach the rmtree without it. Add tests/blocking_io/test_workspace_changes_recorder.py, driving the capture-failure branch and the record finally. Teeth verified per clause under the strict Blockbuster gate: reverting each offload alone reddens its own call (os.path.samestat for rmtree, os.path.abspath for roots/mkdtemp). * fix(workspace-changes): make text-cache prepare handoff cancellation-safe _prepare_capture creates the mkdtemp cache dir inside the to_thread worker, so a run cancelled after mkdtemp but before the coroutine receives the path orphaned the dir. Shield the prepare future and, on cancellation, reclaim its result to remove the dir before re-raising. mkdtemp stays offloaded (the blocking-io gate flags os.mkdir from deerflow code). Adds a deterministic cancellation regression. * fix(workspace-changes): drain repeated cancellation in text-cache reclaim The mkdtemp handoff guard reclaimed the shielded worker's result on the first CancelledError, but the reclaim await was itself cancellable: a second cancel landed there, slipped past `except Exception` (CancelledError is BaseException), and skipped the reclaim while the shielded worker still finished — orphaning the deerflow-workspace-changes-* dir. Repeated cancelled runs accumulate leaks. Move reclaim+remove into a task the caller cannot abandon and drain repeated cancellation until it completes, then restore the cancellation. A repeat cancel interrupts the await, not the task, so the dir is never abandoned; the loop exits only once cleanup has finished, leaving no pending task. Non-cancel paths are unchanged. Adds test_capture_workspace_snapshot_repeated_cancellation_leaks_no_text_cache (double-cancel regression). make test-blocking-io: 43 passed. |
||
|
|
c9b6131f8f
|
fix(skills): reload mounted skills without restarting Gateway (#4264)
* fix(skills): add admin-only reload endpoint * fix(skills): preserve cache when reload fails |
||
|
|
10890e10a8
|
feat(authz): propagate trusted authorization principal context (#4203) | ||
|
|
f9340c1f08
|
test(mcp): cover passive skill tool visibility (#4247)
* test(mcp): cover passive skill tool visibility * test(mcp): tighten deferred discovery coverage |
||
|
|
65afc9b1d2
|
fix(skills): apply allowed-tools only to active skills (#4098)
* fix(skills): scope allowed-tools to active skills * fix(skills): tolerate stale active skill paths * chore: retrigger CI * fix(skills): document policy activation limits * perf(skills): reuse per-step tool policy decisions * fix(skills): harden runtime tool policy contracts * fix(skills): redact cached policy decisions * fix(skills): make slash tool policy authoritative * fix(skills): preserve policy-safe discovery tools * test(skills): cover explicit task delegation policy |
||
|
|
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> |
||
|
|
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. |
||
|
|
de55982c5a
|
fix(subagents): preserve parent checkpoint namespace (#4215)
* fix(subagents): preserve parent checkpoint namespace * test(subagents): align stream isolation coverage |
||
|
|
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.
|
||
|
|
3247f61750
|
fix(middleware): sanitize invalid tool call arguments (#4193)
* fix(middleware): sanitize invalid tool call arguments * refactor(middleware): share tool argument parsing |
||
|
|
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. |
||
|
|
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. |
||
|
|
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 |
||
|
|
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> |
||
|
|
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> |
||
|
|
4e209827f3
|
feat(agent): Add subagent total delegation cap (#4115)
* fix subagent total delegation cap * fix embedded subagent run cap context * fix subagent cap config consistency * fix resumed subagent run cap boundary * fix legacy resume subagent boundary * address subagent cap review feedback --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
|
f85ee590e1
|
docs(backend): add IM channel connections and GitHub agents architecture diagrams (#4112)
Adds two focused architecture docs and references from backend/AGENTS.md: - backend/docs/IM_CHANNEL_CONNECTIONS.md — sequence + graph diagrams for user-owned IM channel connections: bind-code flow (/connect/<start), single-active-owner transfer, provider message routing, owner-scoped file storage. - backend/docs/GITHUB_AGENTS.md — sequence + graph diagrams for GitHub event-driven agents: webhook fan-out, preferred_thread_id = UUID5, GH token lifecycle, race recovery. Co-authored-by: Claude <noreply@anthropic.com> |
||
|
|
5edc7a889e
|
fix(security): neutralize prompt-injection tags in web_capture results (#4099)
The remote-content allowlist in ToolResultSanitizationMiddleware (`_REMOTE_CONTENT_TOOL_NAMES`) covered web_fetch / web_search / image_search but not web_capture, which was added later. The Browserless web_capture tool embeds the target site's `X-Response-Status` reason phrase — free-form text controlled by whatever server is being captured (RFC 7230 §3.1.2) — into its result message via `_target_status_warning`. A malicious page could therefore forge a `<system-reminder>` block (or a `--- END USER INPUT ---` boundary marker) through web_capture that would be escaped for web_fetch, letting attacker-influenced remote content reach the model as authoritative framework context. Add "web_capture" to the allowlist so its result is structurally neutralized for parity with the other remote-content tools. This extends the same defense introduced in #4002 to the one built-in remote-content tool it did not yet cover. Add regression tests that build the web_capture result the way community/browserless/tools.py does (real `_target_status_warning` + `BrowserlessScreenshotResult`) and assert the forged tags/boundary markers are escaped, while a benign status warning is preserved unchanged. |
||
|
|
41658c5ff4
|
feat(skills): add skill review quality gate (#4037)
* feat(skills): add skill review quality gate * fix(skills): skip review eval fixtures in CI * fix(skills): ignore review eval fixtures in bundled scans * fix(skill-review): harden review gate boundaries * fix(skills): address skill review gate feedback |
||
|
|
6389fc03fd
|
fix: ensure visible response after tool runs (#4033)
* fix: ensure visible response after tool runs * fix: clean up terminal response recovery state * fix: bound terminal recovery state |
||
|
|
aafd5077b2
|
feat(subagents): show effective model and token usage on task cards (#4049)
* feat(subagents): show runtime metadata on task cards * fix(subagents): stop task-card render loop and dedupe model fetches Address code review on the runtime-metadata cards: - P1 render loop: the terminal ToolMessage is re-parsed on every MessageList render and always carries modelName/usage, so the presence-based setTasks condition fired a fresh state object each render -> "Maximum update depth exceeded". computeNextSubtask now returns a value-compared `changed` flag and a pure subtaskNotification() routes terminal transitions through the deferred after-render path while skipping no-op re-parses. - Per-card useModels refetch: add staleTime: Infinity to the ["models"] query so every subtask card shares one /api/models fetch instead of refetching on each mount. * make format * refactor(subagents): dedupe token-usage validators + tidy event narrowing Address PR review follow-ups: - DRY: extract one shared token-usage validator per side. Backend status_contract.normalize_token_usage() now backs both the terminal ToolMessage metadata and the subagent.step/.end run events (step_events.py), and frontend messages/usage.normalizeTokenUsage() backs both the live task_running event (lifecycle.ts) and the terminal ToolMessage metadata (subtask-result.ts). Prevents the input/output/ total_tokens validation from drifting across the four former copies. - Nit: onCustomEvent narrows event.type once instead of re-checking the object shape per branch; the redundant task_started early-return (already validated by taskEventToSubtaskUpdate) is dropped. |
||
|
|
c2002d9fac
|
feat(memory): add memory tool sets (#4023)
* feat: add memory-as-tool mode alongside existing middleware mode - Add memory.mode config field (middleware|tool, default middleware) - Add search_memory_facts() for case-insensitive fact lookup - Add 4 memory tools: memory_search, memory_add, memory_update, memory_delete - Wire mode gating in factory.py and lead_agent/agent.py - 256 memory tests passing, zero regressions * fix: harden tool-mode memory scoping and docs * fix: address memory tool mode review feedback * fix(memory): address tool mode review feedback * fix: update config_version to 22 in values.yaml |
||
|
|
bbb3deb231
|
fix(subagents): classify LLM error fallbacks as failed (#4042)
* fix(subagents): classify LLM error fallbacks as failed (#4041) * fix(subagents): address #4042 review on LLM error fallback - clarify _extract_llm_error_fallback docstring: tail-only scan is safe because subagents append their own terminal message, so the last AIMessage is never a stale parent-history marker (cross-ref worker.py) - compute final_result and pop stop_reason only on the COMPLETED branch so the guard stop-reason pop no longer fires on the discarded FAILED path - note the error_detail/"LLM request failed" fallbacks are defensive; the middleware always populates a non-empty content - add regression test locking the stale parent-history marker invariant --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
|
8fbf101de3
|
fix(subagents): inject durable context before compaction (#4040)
* fix(subagents): inject durable context before compaction * fix(subagents): coalesce system messages after durable-context injection Address #4040 review: - append SystemMessageCoalescingMiddleware innermost on the subagent chain so the SystemMessage(authority) DurableContextMiddleware injects is merged into one leading system_message; otherwise the durable fix trades #4039's assistant-first 400 for a duplicate-system 400 on strict backends - add a two-system regression guard driving the real builder output through a strict model; assert exactly one leading SystemMessage - assert single-leading-system in the compaction integration test too - update the middleware count/last-element assertion (coalescer is now unconditionally last, removing the summarization-dependence ambiguity) - compare _skills_root against posixpath.normpath(container_path) - document the coalescer on the subagent chain in backend/AGENTS.md |
||
|
|
d300e3597f
|
Correct memory-config documentation to match the code (#3995)
Two memory-config docs disagreed with the actual defaults/behavior in `config/memory_config.py` and `agents/memory/storage.py`: - backend/AGENTS.md: `staleness_age_days` was documented as "default: 180; range: 1-3650" but the field is `default=90, ge=30, le=365`; `staleness_max_removals_per_cycle` was documented as "default: 5; range: 1-20" but the field is `default=10, ge=1, le=50`. - config.example.yaml: the `storage_path` comment said "Path relative to backend directory", but a relative `storage_path` resolves against the data `base_dir` (`get_paths().base_dir / p`), not the backend working directory, and an absolute path is what opts out of per-user isolation (matching the field's own description in `memory_config.py`). Doc-only change; no behavior change. |
||
|
|
04a85b307a
|
fix(sandbox): scrub abbreviated *_PASS vars and Postgres PGPASSFILE from skill env (#4026)
* fix(sandbox): scrub abbreviated *_PASS vars and Postgres PGPASSFILE from skill env env_policy blocks the full PASSWORD/PASSWD spellings but not the ubiquitous abbreviated _PASS form (DB_PASS, SMTP_PASS, MYSQL_PASS, ...), whose value is the plaintext password itself, so every skill subprocess inherited it. Postgres's file-based credential sources PGPASSFILE (.pgpass locator) and PGSERVICEFILE (pg_service.conf, may carry a password) were likewise missed -- the psql analog of the MYSQL_PWD/REDISCLI_AUTH no-flag sources #4018 added. Collapse *PASSWORD*/*PASSWD* into a single *PASS* pattern (which subsumes both, covers the abbreviated family, and catches PGPASSFILE) and add PGSERVICEFILE to the exact-name denylist. *PASS* deliberately does not touch PWD/OLDPWD, which carry no PASS substring. Capability-handle vars (KUBECONFIG, DOCKER_CONFIG, GIT_ASKPASS, ...) are a broader, non-value-bearing class and are left out of this fix. test_secret_like_names_are_blocked already pins the contract; the new names are added to its parametrize list (red before, green after) and the benign-names test guards against over-scrubbing PWD/OLDPWD. * test(sandbox): pin the *_ASKPASS over-scrub that *PASS* introduces *PASS* also matches the credential-helper vars GIT_ASKPASS, SSH_ASKPASS and SUDO_ASKPASS, which main inherits. Each names a program whose purpose is to hand the caller a credential, so inheriting the pointer is the same leak class as inheriting the value -- a skill can simply invoke it. Scrubbing them is intended; pin all three in test_secret_like_names_are_blocked (red on main) so the behaviour is a decision rather than a side effect of the pattern's shape. test_benign_names_are_allowed could not have caught this: none of its 14 names carries a PASS substring, so it passes identically before and after the broadening and asserts nothing about it. Record that in its docstring -- the absence of a benign PASS-bearing name is a decision, and PWD/OLDPWD are the boundary the list does pin. Also note the incidental COMPASS_*/BYPASS_* over-scrub in the module comment (over-scrubbing is this module's fail-safe direction; required-secrets is the escape hatch), and list PGPASSFILE alongside PGSERVICEFILE in AGENTS.md. |
||
|
|
9097642658
|
feat(memory): add memory consolidation to synthesize fragmented facts (#3996)
* feat(memory): add memory consolidation to synthesize fragmented facts When a fact category accumulates many individual entries, the LLM reviews them during the normal memory-update call (same invocation, no extra API cost) and decides whether groups of related facts can be synthesized into a single richer fact. This completes the memory lifecycle: extraction → guaranteed injection → staleness review → consolidation. - Select fragmented categories by min-facts threshold, surface the most fragmented groups first; prompt-layer caps aligned with apply-layer guardrails so the LLM never sees groups it cannot act on - Cap consolidated confidence at source maximum to prevent inflation; reject results below fact_confidence_threshold - Double-consume protection prevents a fact from being merged into multiple consolidation targets - Feature-gated at both prompt and apply time with per-cycle safety caps - Add 26 tests covering candidate selection, normalization, apply guardrails, and prompt integration * fix(memory): address consolidation correctness issues from PR review Six fixes based on maintainer review of #3996: 1. Deduplicate sourceIds in normalization — ["f1","f1"] previously bypassed the ≥2-distinct-sources check; dict.fromkeys collapses it to ["f1"] which is correctly rejected. 2. Run consolidation after max_facts trim — previously, sources were deleted then the merged fact could be evicted by the trim, leaving no record of either. Moving consolidation last ensures source facts exist in the post-trim index before removal. 3. Fix count= attribute in consolidation prompt — advertised the full category size but listed only max_sources IDs; now uses min(len(group), max_sources) to match what the LLM can act on. 4. Exempt staleness_protected_categories from consolidation candidates — mirrors the existing staleness-review contract so correction facts are never surfaced for merging. 5. Strip and default category in consolidation normalization — " " or " preference " are now normalised, matching _normalize_memory_update_fact. 6. Propagate sourceError from source facts into consolidated fact — correction context is no longer silently lost on merge. * fix(memory): add apply-time guardrails and tests for consolidation P1: mirror the staleness-pass defense-in-depth pattern — build allowed_source_ids from _select_consolidation_candidates at apply time so a protected-category or below-threshold fact proposed by the LLM is rejected regardless of model behavior. P2a: test that LLM-returned confidence is capped at max source confidence and that a capped result below fact_confidence_threshold is rejected. P2b: test that factsToConsolidate with consolidation_enabled=False is a no-op at apply time (35 tests, all pass). * fix(memory): address three correctness issues from second review round 1. Default consolidation_enabled=False — consolidation is lossy (source content is permanently replaced, only consolidatedFrom IDs preserved); new lossy features default to off. config.example.yaml updated to match. 2. Unify confidence coercion between prompt and apply — _build_consolidation_section now calls _coerce_source_confidence(fact) instead of an inline 0.0-default coercion, so a null-confidence fact renders with 0.50 in the LLM prompt and is capped at 0.50 at apply time (same value, same function). 3. Preserve staleness clock on merge — consolidated fact now carries the newest source's createdAt (not now) so aged information does not gain a fresh staleness-review window just by being consolidated; consolidatedAt is added as an explicit audit field. Three regression tests added (default=false, null-confidence consistency, createdAt policy); all guardrail tests now set consolidation_enabled=True explicitly so they test the guardrail, not the feature flag. 38 tests pass. * fix(memory): harden createdAt comparison and confidence handling 1. createdAt max via _parse_fact_datetime — replaces string max() which crashes on non-string createdAt (numeric unix timestamps) and sorts Z/+00:00 mixed formats incorrectly. Mirrors how staleness computes age. 2. Remove dead min(..., 1.0) — _coerce_source_confidence already clamps each source confidence to [0, 1], so max(source_confidences) ≤ 1.0 by contract; the outer min could never bind. 3. Clamp raw_llm_conf to [0, 1] before applying the source cap — out-of- range values like 1.5 are safe today (pinned by the cap) but defensively clamped first so the invariant holds even if the cap is ever loosened. 4. Doc: expand the apply-time guardrails comment to call out the protected- category exclusion via allowed_source_ids — this is the central safety property ("explicit user feedback is never silently merged away"). 5. Test: add test_confidence_fallback_to_max_source_when_llm_omits_field covering the else-branch (LLM omits confidence → uses max_source_conf). 6. Fix lint: reorder imports in test file (stdlib before third-party). 39 tests, all pass. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
|
266883b3dd
|
fix(subagents): inherit summarization middleware and harden step capture (#3875 Phase 3) (#4009)
Phase 3 of #3875 — subagents previously inherited none of the lead's context-compaction, so a deep-research subagent (max_turns up to 150) could accumulate >1M cumulative input before max_turns/timeout/token_budget engaged, even after Phase 2's budget capped the pathological tail. - Gate the subagent runtime chain on the SAME ``app_config.summarization.enabled`` switch the lead reads (per maintainer guidance in #3875), via the shared ``create_summarization_middleware`` factory. One config covers both chains; no separate ``subagents.summarization`` field. No-op when summarization is off (factory returns None). - ``skip_memory_flush=True`` on the subagent path: the factory otherwise attaches ``memory_flush_hook`` (when memory.enabled), which flushes pre-compaction messages into durable memory keyed by thread_id. Subagents share the parent's thread_id, so without skipping the hook a subagent's internal turns would pollute the PARENT thread's durable memory (#3875 Phase 3 review point). - Harden ``capture_new_step_messages`` to tolerate history contraction: summarization rewrites the messages channel via ``RemoveMessage(id=REMOVE_ALL_MESSAGES)``, shrinking len(messages) below the step-capture cursor. Without a reset, every step appended after the compaction point was dropped until length overtook the stale cursor (#3845 interaction, maintainer validation point (a)). Cursor now resets to the new tail; id/content dedup prevents re-emitting pre-compaction steps. - Couple the DEFAULT token-budget ceiling to ``summarization.enabled`` (#3875 Phase 3 review point): 1M when compaction is on, 2M when off (preserves Phase 2's deliberate headroom for summarization-off deep-research runs that can exceed 1M). A user-set budget (global or per-agent) always wins regardless of the switch. Flagged tunable. The summarization middleware does not implement ``consume_stop_reason``, so the Phase 2 guard-cap stop-reason channel is unaffected. Refs: https://github.com/bytedance/deer-flow/issues/3875 |
||
|
|
b06372b812
|
feat(backend): queue rapid same-thread messages and preserve topic card previews (#3988)
* feat(backend): queue rapid same-thread messages and preserve topic card previews * fix: checkstyle * refactor(channels): make feishu serialization policy-driven and fix queue cleanup |
||
|
|
ebc09ce130
|
feat(mcp): auto-promote deferred MCP tools from routing hints (#4019)
* feat(mcp): auto-promote deferred MCP tools from routing hints
When tool_search.enabled=true defers MCP tool schemas, PR1 routing hints
still require the model to spend a tool_search discovery round trip before
it can call the tool the routing metadata already points at. This adds a
McpRoutingMiddleware that matches the latest user message against PR1
routing keywords and promotes the matching deferred schemas before the
model call, removing that round trip.
Design (soft routing, opt-in, additive):
- Matches only the latest real HumanMessage (shared is_real_user_message
helper, reused by SkillActivationMiddleware so the two cannot drift);
case-insensitive substring match, no tokenizer dependency.
- Ordering: priority desc, then tool name asc; capped by the new global
tool_search.auto_promote_top_k (default 3, clamped 1..5). Does not add or
consume a per-tool auto_promote_top_k (PR1 schema unchanged); a per-tool
value is ignored with a DEBUG note.
- Returns a plain {"promoted": ...} state update (not a Command) and relies
on ThreadState.merge_promoted for union/dedupe, so auto-promote and a
model-triggered tool_search converge on the same catalog hash.
- Installed before DeferredToolFilterMiddleware on every deferred-tool path
(lead agent, subagent, embedded client, webhook via shared builders);
a construction-time assert rejects the reversed order. catalog_hash is
None / no routing index is a complete no-op, so bootstrap and ACP skip it.
- Privacy: never executes tools, never promotes policy-filtered tools, adds
no routing keywords or matched tool names to trace metadata or INFO/WARN
logs.
No behavior change when tool_search.enabled=false.
Tests: index construction, matching semantics, middleware state updates,
same-cycle deferred-filter interaction, lead/subagent/embedded-client
builder wiring + order invariant, config clamping, config.example.yaml
parseability, and privacy assertions.
* refactor(mcp): address auto-promote review nits
- executor: access app_config.tool_search.auto_promote_top_k directly to match
the lead-agent and embedded-client paths (drop the over-defensive getattr that
masked missing config); update the subagent test mock to carry tool_search.
- tool_search / mcp_routing_middleware: cross-reference the duplicated routing
priority/keyword normalization between the builder and the middleware's
defensive _normalize_index so they cannot silently drift.
- MCP_SERVER.md: document that auto-promote keyword matching is a case-insensitive
substring test (not word-boundary), advising distinctive keywords.
|
||
|
|
66c0c641bf
|
fix(gateway): live-tail malformed redis reconnect ids (#4012)
* fix(gateway): live-tail malformed redis reconnect ids * test: avoid redis live-tail heartbeat gaps * fix: handle terminal malformed redis reconnect ids |
||
|
|
c30c8ef759
|
fix: recover from empty tool call names (#4008)
* fix: recover from empty tool call names * test: harden empty tool call recovery --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
|
5ba25b06ec
|
feat(mcp): add MCP routing hints (#4004)
* feat: add MCP routing hints * test: isolate mcp routing prompt config * fix: address mcp routing review feedback |
||
|
|
b36d7194d9
|
fix(security): neutralize prompt-injection tags in remote tool results (#4002)
* fix(security): neutralize prompt-injection tags in remote tool results
User input is already neutralized for framework/injection tags, but tool
results are not. Remote content fetched by web_fetch/web_search is equally
untrusted and can carry a forged <system-reminder> block that reaches the
model verbatim as authoritative context.
Extract a shared neutralize_untrusted_tags() primitive from
InputSanitizationMiddleware and apply it to remote-content tool results
(web_fetch/web_search/image_search) via a new ToolResultSanitizationMiddleware.
Local tool output (bash/read_file) is left untouched so legitimate code/file
content is never mangled.
* test: update subagent middleware count for tool-result sanitizer
The new ToolResultSanitizationMiddleware adds one entry to the shared runtime
chain (11 -> 12). Update the subagent count assertion, use a lazy import for
neutralize_untrusted_tags so the module loads even when tests stub the
input-sanitization module, and document the new middleware in AGENTS.md.
* fix(security): address review — sanitize bare str list items; document MCP scope
- Neutralize bare str elements inside a ToolMessage content list (previously
only {type:text} dict blocks were rewritten), matching the str-in-list shape
ToolOutputBudgetMiddleware._message_text already anticipates.
- Document the name-based allowlist limitation: MCP remote-content tools
registered under arbitrary names (e.g. fetch_url) are not covered; a name
heuristic is avoided to prevent mangling local tool output, with metadata
tagging tracked as a follow-up. Add a regression test pinning this boundary.
|
||
|
|
c9fb9768d4
|
fix(subagents): unify guardrail caps on additive stop_reason + add token_budget (#3875 Phase 2) (#3980)
Phase 2 of #3875. Two guardrail axes can end a subagent run early — the turn budget (GraphRecursionError) and the token budget (TokenBudgetMiddleware) — and both now surface *why* through one additive `subagent_stop_reason` field instead of a status enum. This completes and course-corrects Phase 1 (#3949), which shipped the turn-budget cap as a `max_turns_reached` status enum. The agreed Phase 2 design replaces that enum with an optional `stop_reason` field (token_capped | turn_capped | loop_capped): a new enum value would break v1 consumers, while an additive field is ignored by older frontends and ledger readers. `max_turns_reached` and SubagentStatus.MAX_TURNS_REACHED are removed. - subagents.token_budget config (default enabled, 2,000,000 tokens, warn 0.7) with per-agent override; TokenBudgetMiddleware is now attached in build_subagent_runtime_middlewares so the cost-ceiling backstop engages for every subagent. The hard-stop does not raise — it strips tool_calls and lets the run finish with a final answer, recording the cap on a per-run consume_stop_reason() accessor. - executor.py: on normal completion it reads consume_stop_reason() and stamps completed + token_capped when the budget fired; on GraphRecursionError it recovers the last AIMessage partial (completed + turn_capped) or, if nothing usable survived, failed + turn_capped. SubagentResult gains stop_reason. - status_contract.py / contracts/subagent_status_contract.json (v2) / frontend subtask-result.ts: additive subagent_stop_reason field, pinned by test_status_values_match_contract / test_stop_reason_values_match_contract. - task_tool.py + delegation_ledger.py: drop the max_turns_reached paths; the ledger captures stop_reason and renders model-facing "capped" guidance so the lead reuses a capped completion knowingly. The 2,000,000-token default is deliberately loose (tighten to taste) — it would have roughly halved the reported 4.4M burn while leaving legitimate deep-research runs (max_turns=150) room. Subagent summarization is a follow-up. |
||
|
|
01dc067997
|
feat: add composer input polishing (#3986)
* feat: add composer input polishing * Revert "Merge branch 'main' into feat/input-polish" This reverts commit 5b6ceccf0db3092bc62fde3b05e7816829601756, reversing changes made to 45fbc57fef5fa5fd878cf0176c37f3e3bc7ebef6. * Merge main into feat/input-polish * style(frontend): format input helper polish guard * fix(input-polish): address composer polish review findings Frontend - Add a cancel affordance to the in-flight polish status pill that calls abortInputPolishRequest(), so a slow/hung provider no longer hard-locks the composer for up to stream_chunk_timeout with a page reload (and draft loss) as the only escape. - Reset promptHistoryIndexRef/promptHistoryDraftRef when a rewrite is applied (and on undo), so a stale history-browse index can no longer let the next ArrowDown silently overwrite the polished draft. - Disable polishing while an open human-input card is present, matching the frontend/AGENTS.md rule that composer entry points defer to the card so card-reply metadata is preserved. - canPolishInput now reuses parseGoalCommand/parseCompactCommand instead of a third hardcoded reserved-command regex, and drops the phantom /help entry (no /help parser exists in the composer), so future builtins only need to be taught to the existing parsers. Backend - Extract the non-graph one-shot LLM path (build model + inject Langfuse metadata + system/user invoke + text extract) into deerflow.utils.oneshot_llm.run_oneshot_llm, shared by the input-polish and suggestions routers so tracing-metadata and invocation shape cannot drift between the two copies. - strip_think_blocks gains truncate_unclosed (default True, preserving the suggestions/goal JSON-prep behavior); input polish passes False so a draft that legitimately contains a literal <think> substring is no longer truncated into a partial rewrite or a spurious 503. - Validate the empty-check and max_chars boundary against the same stripped view of the draft that is sent to the model, so the user-facing length boundary and the model input can no longer disagree. Tests / docs - Backend: literal-<think> preservation, whitespace-only rejection, and normalized-length/model-input agreement cases; suggestions tests repoint the create_chat_model patch to the shared helper module. - Frontend: helper unit tests updated for the /help/reserved-command change; a new Playwright case covers cancelling an in-flight polish request. - backend/AGENTS.md documents the shared one-shot helper and the polish normalization/think-tag behavior. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
|
e3137b13cf
|
fix(provisioner): gate legacy skills mount by user visibility (#3985)
* fix(provisioner): gate legacy skills mount by user visibility * fix(test): aio sandbox provider * fix: use shared legacy skill visibility helper for sandbox mounts |
||
|
|
658c39ccf7
|
feat(skills): Add native SkillScan phase 1 for skills (#3033)
* Add phase 1 skill static scanning * Rework SkillScan phase 1 as native scanner * refactor(skillscan): align phase 1 with trimmed RFC contract - SecurityFinding: 7 fields (rule_id, severity, file, line, message, remediation, evidence); category/analyzer derive from the rule_id prefix, confidence/column/fingerprint/metadata removed - scan_archive_preflight()/scan_skill_dir() are pure functions: no ScanContext, no policy schema; CRITICAL-blocks is a code constant and skill_scan.enabled is applied by enforce_static_scan()/callers - secret-* evidence is redacted before findings leave the scanner - de-dup keys on (rule_id, file, line) so repeated occurrences keep distinct locations for agent self-correction - cloud-metadata detection consolidated into network-cloud-metadata - nested zip members get a one-level stdlib magic-byte peek; an executable member escalates package-nested-archive to CRITICAL - install metadata sidecar removed (Phase 7 decides if it is needed) - rule specs moved next to their analyzers; skillscan/rules/ removed - tests updated + new anchors: redaction, dedup lines, nested-zip escalation, single cloud-metadata rule, bundled-skill zero-CRITICAL * fix(skillscan): tighten reverse-shell/secret/archive scan rules from review Address PR #3033 review feedback on the native SkillScan analyzers: - Reverse-shell false positives: split shell detection by signal strength (/dev/tcp/, nc -e stay CRITICAL; bash -i, mkfifo -> new HIGH shell-reverse-shell-heuristic, warn->LLM). The Python check is now AST-anchored on real socket.socket/os.dup2/subprocess call sites instead of raw-text substring matching, so prose/docstrings no longer hard-block. - Secret evidence: _redact_secret_evidence returns [redacted] with no secret bytes (was value[:6], which leaked 2 real token bytes past the prefix). - Archive DoS: cap outer archive member count (_MAX_ARCHIVE_MEMBERS=4096); scan_archive_preflight early-aborts with a package-too-many-members CRITICAL finding (routes through the existing blocked->400 fail-closed path). - shell-destructive-command: broaden the rm -rf matcher to sensitive system roots (/home, /usr, /*, --no-preserve-root /) while leaving safe subpaths unflagged. - Dead code: collapse _decode_text_for_analysis to a single decode path and drop the unused _TEXT_SUFFIXES set and _has_text_shebang helper. - local_skill_storage: document why the host_path branch keeps app_config possibly-None (lazy kill-switch resolution; avoids eager get_app_config in config-free environments such as CI). Tests: new negative/positive coverage in test_skillscan_native.py. Full backend suite 6616 passed, 26 skipped. |
||
|
|
26d7a5970d
|
feat: add manual context compaction (#3969)
* feat: add manual context compaction * fix: harden manual context compaction |
||
|
|
8fbcdf821b
|
Fix/card tool message bug (#3976)
* fix(runtime): add final reconciliation for missed tool messages * fix(gateway): persist hidden human input card responses Persist allowlisted hidden human_input_response messages in RunJournal so Human Input Cards can recover answered state from run_events after checkpoint compaction. Keep generic internal hidden messages filtered and add regression coverage for ask_clarification responses. |
||
|
|
47b0f604f4
|
feat(frontend):enhance the ask_clarification interaction with visualized card (#3956)
* feat(frontend): add structured human input cards for ask_clarification Implement a reusable Human Input Card flow for ask_clarification while keeping the existing text fallback for older clients and IM channels. Backend: - Add structured ToolMessage.artifact.human_input payloads for clarification requests. - Preserve ToolMessage.content as the readable Markdown/text fallback. - Normalize clarification options from native lists, JSON strings, plain strings, mixed scalar values, None, and missing options. - Derive input_mode as choice_with_other when options exist, otherwise free_text. - Keep disable_clarification non-interactive behavior as a plain ToolMessage with no human_input artifact. - Cover artifact persistence and Gateway message metadata preservation in tests. Frontend: - Add human input protocol types, runtime guards, extractors, response builders, and thread-state helpers. - Add reusable HumanInputCard with option buttons, free-text input, pending, read-only, disabled, and answered states. - Render structured clarification cards from artifact.human_input, with Markdown fallback for malformed or legacy tool messages. - Preserve line breaks in structured question/context/option text. - Hide submitted clarification bridge messages from the chat UI via additional_kwargs.hide_from_ui. - Send structured human_input_response metadata through the fourth sendMessage options argument, preserving run context in the third argument. - Wire submissions for normal chats, custom agent chats, agent bootstrap chats, and sidecar chats. - Derive answered state from raw thread.messages so hidden replies still update the original card. - Clear pending state when the hidden reply arrives, dispatch is dropped, or a later async stream failure appears on thread.error. * perf(frontend): optimize HumanInputCard UI interactions - Support Enter key to submit text input (Shift+Enter for newline) - Render question and context fields as Markdown instead of plain text - Replace deprecated FormEventHandler type with structural typing * test(frontend): add unit test cover optimize HumanInputCard UI interactions * feat(frontend): disabled chatbox when has new human-input-card * fix(style): lint error fix * fix: sanitize hidden human input replies - Preserve IME composition safety for human input card Enter submits - Treat hidden human input responses as genuine user messages for sanitization - Keep hidden card replies in memory filtering while excluding malformed/internal hidden messages - Add regression coverage for card IME handling and hidden reply sanitization * fix: tighten human input response validation - Reject empty hidden human input response values - Remove invalid list ARIA role from human input card options - Add backend coverage for empty response payloads --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
|
fd41fdb065
|
feat(middleware): add structured tool result meta and tool-progress state machine (#3601)
* feat(middleware): add structured tool result meta and tool-progress state machine
feat:
- Add tool_result_meta.py: ToolResultMeta dataclass (status/error_type/retryable/
recoverable_by_model/recommended_next_action/source) + normalize_tool_result and
stamp_exception_meta utilities; classifies every ToolMessage regardless of path
- Add ToolProgressMiddleware: per-(thread_id, tool_name) state machine ACTIVE →
WARNED (hint injected as HumanMessage) → BLOCKED (call short-circuited); Jaccard
near-duplicate detection for repeated successful results; auth/config/internal
errors bypass WARNED and go directly to BLOCKED; LRU-bounded thread state store
- Add ToolProgressConfig: all thresholds configurable (stagnation_threshold,
warn_escalation_count, jaccard_similarity_threshold, exempt_tools, etc.);
disabled by default (enabled: false)
- Wire ToolProgressMiddleware as outer wrapper around ToolErrorHandlingMiddleware
in _build_runtime_middlewares so it receives results already carrying
deerflow_tool_meta
fix:
- ToolErrorHandlingMiddleware now calls stamp_exception_meta on exception path and
normalize_tool_result on success path so every ToolMessage carries deerflow_tool_meta
test:
- Add test_tool_result_meta.py: 26 cases covering all classification paths,
stamp_exception_meta, and normalize_tool_result Command passthrough
- Add test_tool_progress_middleware.py: 27 cases including full async paths,
Jaccard duplicate detection, LRU eviction, hint injection, and malformed meta
passthrough
- Extend test_tool_error_handling_middleware.py: middleware ordering invariant and
meta stamping on exception
docs:
- Add tool_progress section to config.example.yaml with all fields and descriptions
- Update CLAUDE.md middleware chain documentation (entries 8-9)
Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
* fix(middleware): recoverable errors stay WARNED; fix auth keyword shadowing
fix:
- WARNED is terminal for recoverable_by_model=True errors (no_results, not_found,
permission); hint re-injected on each problem call instead of escalating to
BLOCKED, so the model can retry with different parameters (e.g. fresh query,
new URL) without being hard-blocked by a prior stagnation count.
Non-recoverable (rate_limited, transient) still escalate WARNED → BLOCKED
after warn_escalation_count more problems; auth/config/internal remain
immediately BLOCKED.
- Remove bare "api key" keyword from auth classification rule so "no api key
configured" correctly classifies as config (not auth), producing the accurate
block-reason text for the model.
docs:
- CLAUDE.md: document all three ToolProgressMiddleware transition paths
- config.example.yaml: update inline state-machine comment to match new paths
test:
- test_recoverable_errors_stay_warned_indefinitely: WARNED never escalates for
recoverable errors regardless of how many problem calls accumulate
- test_recoverable_error_re_injects_hint_past_escalation: hints continue past
the escalation zone for recoverable errors
- test_no_api_key_is_config_not_auth: regression guard for keyword shadowing fix
Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
* fix(tool_result_meta): add JSON error extraction and fix source classification
fix:
- Fix non-standard error path: source was "exception" but should be "tool_return"
- Add _extract_json_error_text to isolate JSON error fields from noisy JSON bodies
(e.g. Brave Search {"error": "...", "query": "..."} — query keywords no longer
pollute error classification)
- Add success-path JSON extraction to catch tools that return HTTP 200 with a JSON
error body (status="success" but {"error": "API key not configured"})
- Add _SEMANTIC_ZERO_ERROR_STRINGS frozenset to suppress false positives from tools
that use {"error": "none"} / {"error": "null"} / {"error": "ok"} as success signals
- Document that stamp_exception_meta always overwrites existing TOOL_META_KEY
(exception-derived classification is authoritative over tool return-time stamps)
test:
- Add parametrized regression tests for all semantic-zero error strings
- Add tests for non-standard error path source field
- Add tests for JSON error extraction (nonstd, success-path, numeric, falsy values)
- Correct test comment for test_no_api_key_is_config_not_auth
Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
* fix(tool_progress_middleware): fix 6 bugs, add terminal guard and structured logging
fix:
- H1: fix exempt_tools empty-set silently ignored — use `is not None` instead of
truthiness check so ToolProgressConfig(exempt_tools=set()) correctly disables all
exemptions
- Fix _get_block_reason creating phantom LRU entries via _get_state (write path);
now uses dict.get + explicit move_to_end on read path only
- Fix _pending memory leak: LRU eviction of _phase_states now synchronously removes
all (evicted_thread, *) keys from _pending
- Fix _assess_and_transition missing terminal guard for blocked state — a recoverable
error result could silently demote blocked → warned in concurrent-race scenarios;
early return preserves terminal semantics
- Fix recent_word_sets window: stored [-5:] but is_near_duplicate only compared [-3:];
align to [-3:] and change type list→tuple (prevents accidental in-place mutation
across dataclasses.replace shallow copies)
- Fix _format_hint missing "success" key and "continue" action: Jaccard near-duplicate
results produced the generic fallback instead of a specific actionable message
feat:
- Add structured state-transition logging (ACTIVE/WARNED/BLOCKED transitions, blocked
intercepts, hint injection debug log)
test:
- Add regression tests for all 6 bug fixes (H1, phantom LRU, pending leak, terminal
guard, window alignment, format_hint near-dup)
- Add Jaccard near-threshold boundary test (7/9 vs 8/9 Jaccard)
- Add production min_words=10 skip test for short content
- Add exempt_tools empty-set and None round-trip tests
- Add _augment_request deduplication test
- Add before_agent current-run preservation test
- Add structured logging tests (WARNED/BLOCKED/ACTIVE/intercepted/debug)
Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
* chore(config): remove unused backward-compat fields from ToolProgressConfig
Remove max_calls_per_intent and window_size fields that were marked
"Retained for backward compatibility; not used by the current state machine"
when the state machine was introduced. Pydantic v2 ignores unknown fields
by default, so existing config.yaml files with these keys remain valid.
Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
* fix(tool_progress): address PR review and multi-agent review findings
fix:
- Remove <80-char length gate for partial_success; only _PARTIAL_MARKERS now
- Add word-boundary regex for numeric HTTP codes (401/403/404/500) to avoid
false positives like "500ms" or "4010 rows" triggering hard-block
- Add "task" to default exempt_tools (delegation primitive, not a search tool)
- Remove move_to_end() from _get_block_reason read path; blocked threads were
permanently warm in LRU, starving active threads of eviction slots
- Add _reset_blocked_states in before_agent: scope BLOCKED and WARNED states
to a single run; clear recent_word_sets so stale Jaccard windows don't cause
false near-duplicate detections in the next run
- Compute word_set() lazily (only for success results); cap content at 8192
chars to bound memory and CPU cost on large tool results
- Remove unused retryable field from ToolResultMeta (no consumer existed)
- Add isinstance-based ordering guard and warning log for missing meta
- Fix JSON-without-error-key fallback: use _UNKNOWN_ERROR instead of
classifying incidental field values (e.g. {"user_id": 401} → auth → stop)
- Fix _extract_json_error_text: use json.dumps for dict/list error fields
instead of str() which produced Python repr matching config rules spuriously
- Add "no results found"/"no content found"/"no images found" to _PARTIAL_MARKERS
so success responses with empty results trigger stagnation detection
- Fix immediate-block path to increment consecutive_problems (was left at 0)
- Fix _queue_assessment: skip phantom _pending entries for evicted threads
- Bump config_version 13→16 (upstream added 14/15; tool_progress is additive)
test:
- Update test_short_content_is_partial → test_short_terse_success_is_not_partial
- Add parametrized test_numeric_keyword_word_boundary (8 positive + negative cases)
- Add test_before_agent_resets_blocked_states_for_new_run (strengthened assertions)
- Add test_before_agent_resets_warned_states_for_new_run
- Add test_missing_meta_on_non_exempt_tool_emits_warning
- Add test_middleware_ordering_guard_raises_when_progress_is_inner
- Add test_auth_error_immediately_blocked asserts consecutive_problems == 1
- Add tests for JSON-without-error-key, dict error field, no-results partial_success
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(tool_progress): address second PR review — perf, architecture doc, concurrency note
fix:
- Extract content.lower() once before _PARTIAL_MARKERS check in normalize_tool_message;
previously computed up to 7× per call inside the generator (once per marker)
docs:
- Add division-of-labor paragraph to ToolProgressMiddleware module docstring explaining
coexistence with LoopDetectionMiddleware: result-quality guard (per-tool BLOCK) vs
call-pattern guard (whole-turn hard-stop); no shared state, no double-stop risk
- Add threading.Lock comment explaining why asyncio.Lock is not used (short critical
sections, must also protect sync wrap_tool_call path from subagent executor threads)
- Update backend/CLAUDE.md entry 8 with division-of-labor summary; fix entry 9
(remove stale retryable field reference, add missing recoverable_by_model/source)
test:
- Add test_tool_progress_and_loop_detection_coexist_without_interfering: drives both
middlewares to WARNED state simultaneously, verifies independent state, independent
hint queues, and no cross-contamination; uses snapshot copy for final assertion
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(tool_progress): reset all tool states at run boundary; fix semantic-zero test validity
fix:
- _reset_run_states (formerly _reset_blocked_states) drops the phase filter and
resets all tracked (thread, tool) pairs unconditionally at before_agent; ACTIVE
tools with sub-threshold consecutive_problems or cached recent_word_sets no longer
bleed into the next run, preventing spurious WARNED transitions on clean R2 calls
- test_normalize_json_semantic_zero_error_string_not_treated_as_error: replace
{error_value!r} f-string (produces invalid JSON with single quotes) with
json.dumps so _extract_json_error_text actually parses the payload and the
_SEMANTIC_ZERO_ERROR_STRINGS guard is exercised, not bypassed at json.loads
test:
- add test_before_agent_resets_active_state_consecutive_problems_and_word_sets to
lock the ACTIVE-phase run-boundary reset: drives tool to active/cp=1/ws≠() in R1,
asserts both fields are zero/empty after before_agent fires for R2
* docs(tool_progress): document intentional per-run reset vs LoopDetection thread-scoped retention
Addresses reviewer observation in PR #3601 that _reset_run_states diverges
from LoopDetectionMiddleware's cross-run scoping policy without explanation.
Expands the _reset_run_states docstring to record the intentional design
choice: ToolProgressMiddleware resets per-run because result-quality errors
(rate_limited, transient) are time-bound and may resolve between turns —
retaining stale counters would risk false-positive BLOCKED calls.
LoopDetectionMiddleware retains history across runs because call-pattern
loops are time-invariant. The divergence is by design, not oversight.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(middleware): restore ReadBeforeWriteMiddleware as outermost write gate
A merge conflict resolution had accidentally placed ReadBeforeWriteMiddleware
after ToolErrorHandlingMiddleware (inner), reversing the original intent from
b81334cc where it was the outermost write gate before ToolErrorHandling.
fix:
- Restore ReadBeforeWriteMiddleware to outer position: ReadBeforeWrite →
ToolProgress → ToolErrorHandling. Blocked writes now return immediately
without consuming a ToolProgress slot.
- Add normalize_tool_result call on blocked ToolMessages so they carry
deerflow_tool_meta (recoverable_by_model=True) even though they bypass
ToolErrorHandlingMiddleware.
test:
- Add test_blocked_write_has_deerflow_tool_meta (sync + async) to lock the
normalize_tool_result behavior on blocked writes.
- Fix chain order assertions in TestChainWiring and
test_build_lead_runtime_middlewares_chain_order_matches_agents_md.
docs:
- Renumber AGENTS.md items: 10→ReadBeforeWrite, 11→ToolProgress,
12→ToolErrorHandling; update descriptions to reflect outermost-gate design.
- Fix stale cross-reference: LoopDetectionMiddleware (item 23) → (item 25).
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4 <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
|
||
|
|
28f2b07b79
|
feat(memory): add staleness review to prune silently-outdated facts (#3860)
* feat(memory): add staleness review to prune silently-outdated facts
Facts created long ago may become outdated without any future conversation
explicitly contradicting them ("Silent Staleness"). This adds a staleness
review mechanism that surfaces aged facts to the LLM during the normal
memory-update call so it can semantically judge whether each is still valid.
- New MemoryConfig fields: staleness_review_enabled, staleness_age_days,
staleness_min_candidates, staleness_max_removals_per_cycle,
staleness_protected_categories
- New STALENESS_REVIEW_PROMPT section injected into MEMORY_UPDATE_PROMPT
when enough stale candidates exist
- New staleFactsToRemove output field in the LLM response schema
- Safety cap limits max removals per cycle, keeping lowest-confidence
entries when the LLM returns more than the cap
- Correction facts (category=correction) are protected by default
- Observability via structured logging of each removal with reason
- 32 unit tests covering parsing, selection, triggers, formatting,
normalization, safety cap, and integration
* fix(memory): add deterministic guardrail for staleness removals
_apply_updates previously removed any fact id the LLM returned in
staleFactsToRemove without verifying it was in the actual staleness
candidate set. An LLM slip could silently delete protected-category
facts (e.g. correction) or fresh facts, defeating the stated guarantee.
Now intersect stale_ids_to_remove with _select_stale_candidates before
the safety cap, making the protection independent of both model behavior
and the staleness_review_enabled flag.
Add three regression tests:
- test_protected_category_fact_refused_at_apply
- test_non_aged_fact_refused_at_apply
- test_guardrail_runs_when_staleness_review_disabled
* docs(memory): sync AGENTS.md staleness config + simplify datetime parsing
Address reviewer feedback from PR #3860:
- Add staleness workflow step and 5 new config fields to backend/AGENTS.md
- Simplify _parse_fact_datetime: drop manual Z→+00:00 replace, Python 3.12+ fromisoformat handles Z natively
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
|
||
|
|
eb5eb9c574
|
fix(docker): keep config mounts stable across host saves (#3954)
* fix(docker): keep mutable config mounts stable * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> |