mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-07-27 08:28:00 +00:00
251 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
阿泽
|
1baa8ad696
|
feat(clarification): structured form fields for human-input cards (#4400 Phase 1) (#4406)
* feat(clarification): structured form fields for human-input cards Add a request-side v2 `form` mode to the ask_clarification protocol so business flows (e.g. expense reimbursement) can collect several values in one card instead of sequential free-text questions: - `ask_clarification` gains a restricted `fields` parameter (text / textarea / number / select / multi_select / checkbox / date) - ClarificationMiddleware validates and normalizes fields explicitly (whitelisted types, unknown -> text, select-likes without options -> text, duplicate/invalid entries dropped, all-invalid falls back to the legacy modes) since the middleware short-circuits before tool execution; the plain-text fallback lists fields for IM channels - Form payloads carry `version: 2` so older frontends degrade to the text fallback; replies stay on the v1 response protocol — the card submits a readable summary as `response_kind: "text"`, so journal persistence and answered-card recovery are unchanged - Frontend renders typed field controls with required-field validation and compact multi-select chips Part of #4400 (scope narrowed per maintainer feedback: request-side only, no new response kinds, no top-level multi_choice). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(clarification): harden form protocol per review feedback Address the five review points on #4406: - Reject field names colliding with JS Object.prototype members on both sides; frontend reads form values via own-property access only, so `constructor`/`toString`-style names can no longer leak inherited members into required validation or the submitted summary - Close open requests answered through the legacy text fallback: a visible plain human reply (no response metadata) now marks every previously-opened request as answered, so upgrading to a v2-aware frontend cannot leave the composer locked on an already-answered card - Give checkbox fields deterministic boolean semantics: values are seeded to an explicit false ("no" in the summary) and `required` means must-agree/consent; documented in the tool schema - Make middleware field validation atomic: structurally broken entries (bad/duplicate/reserved names, over-cap field/option counts or text lengths) degrade the whole form instead of silently dropping fields; options are trimmed/deduped with blanks removed so the backend never emits payloads the frontend parser rejects - Associate form labels/controls (htmlFor/id), aria-required, aria-invalid, and error descriptions for accessibility Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(clarification): type the fields item schema via TypedDict Replace `fields: list[dict[str, Any]]` with `list[ClarificationFormField]` (a TypedDict with `name` required and the type whitelist as a Literal) so the provider-facing tool schema documents the item shape instead of an opaque object relying on the docstring. Runtime validation is unchanged and stays in ClarificationMiddleware, which intercepts the call before tool execution. Addresses the non-blocking review suggestion on #4406. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(frontend): drop unsupported aria-invalid from multi-select group jsx-a11y: role=group does not support aria-invalid; the error linkage stays via aria-describedby. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(clarification): coerce numeric required flags and normalize fields once - `_normalize_bool` now coerces 1/0 (some providers serialize booleans as integers), so `required: 1` no longer silently flips to optional - `_handle_clarification` normalizes `fields` once and passes the result to both the text fallback and the payload builder Addresses the non-blocking review nits on #4406. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(clarification): harden form protocol per contract review round 2 Backend: - Guard unhashable JSON in the intercept path: `type: []`/`{}` degrades the field to text and `clarification_type: []` coerces to str instead of raising TypeError (which, with return_direct, ended the turn with an error and no card or fallback) - Add a total budget over the serialized normalized fields (16KB UTF-8 bytes): per-item caps alone admitted forms whose IM text fallback exceeded channel delivery limits (Slack 40k chars, Feishu ~30KB card), silently truncating trailing fields; a boundary test proves any accepted form's fallback stays deliverable Frontend: - Submission value now appends a JSON block keyed by stable field names (readable summary alone is delimiter-ambiguous), with a collision regression test - Parser boundary tightened to match backend constraints: empty option values (Radix SelectItem crash), duplicate option ids/values, duplicate field names, and the form<->version-2 binding are rejected - Keep the error node mounted while any field is still invalid so aria-describedby never points at a removed element (happy-dom interaction test) - Required semantics are now accessible: native checkbox control (no HTML required attribute — it would intercept the custom submit path), visually-hidden localized "required" markers next to the aria-hidden asterisks - Legacy-fallback closure narrowed to the latest unanswered request: nothing guarantees a single outstanding clarification across runs, and closing all would silently swallow older decisions; an older request left open becomes the active card again Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(frontend): keep clarification selects controlled --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Vanzeren
|
e01173d8b2
|
bench(checkpoint): production-shaped full/delta benchmark with configurable snapshot frequency (#4467)
* feat(checkpoint): production-shaped full/delta benchmark with configurable snapshot frequency - Group benchmark scripts into per-family folders (checkpoint/, sandbox/) - Extract shared benchmark infrastructure into checkpoint_bench_common.py - Add checkpoint_delta_snapshot_frequency config (default 1000, process-frozen); freeze it in make_lead_agent and DeerFlowClient; key the state-schema adaptation cache by resolved frequency - New bench_production.py: per-case child processes run N ainvoke turns through the real lead-agent graph (scripted deterministic model, real AsyncSqliteSaver), then measure GET /state + POST /history through the real Gateway route stack in one event loop (httpx ASGITransport), cold/warm accessor-cache split, cross-mode digest gates - New summarize_production.py: delta/full ratios plus decision metrics (snapshot_write_spike, cache_effect_ms, checkpoint_write_share, auto-discovered history per-limit ratios) * fix(checkpoint): address production benchmark review |
||
|
lllyfff
|
8145d66a33
|
feat(memory): memory message processing (#4447)
* feat(memory): signals-based update pipeline + always-on watermark/trivial filter
Refactor the DeerMem memory update pipeline (message_processing -> queue ->
updater) around a signals frozenset seam, replacing the
(filtered, correction_detected, reinforcement_detected) 3-tuple with
(filtered, signals: frozenset[str]) end to end.
message_processing:
- Externalize signal-detection patterns to YAML (message_patterns/*.yaml).
- Extend signals from correction/reinforcement to a 6-class set
(correction/reinforcement/preference/identity/goal/decision); detect_signals
returns a frozenset aligned with the fact category enum.
- Pure-acknowledgment turns ("ok"/"好的"/...) are always filtered out before
enqueue (whole-message fullmatch), saving an extraction LLM call.
queue (core/queue.py):
- In-memory list + debounce timer, with flush_sync (graceful-shutdown drain
that joins an in-flight worker under a hard timeout) and queue_max_depth
backpressure (signal-bearing updates always admitted; QueueFull otherwise).
- Same-key updates coalesce with a signal union; per-batch success/fail summary.
updater (core/updater.py):
- head500+tail500 message truncation (replaces the 1000-char head chop).
- Always-on per-thread watermark: feed only messages added since the last
extraction. The watermark is in-memory and is not advanced on failure, so a
failed/lost update is re-fed on the next conversation turn.
- [MANUAL] prompt marker for user-authored facts (source.type="manual").
- Post-invoke extraction_callback (host-injected) emitting facts_extracted /
facts_accepted / rejected_low_confidence; the host default logs metrics and
flags >60% rejection.
Confidence filtering remains in _apply_updates (the existing
fact_confidence_threshold check); there is no separate write gate.
Consolidation stays opt-in (lossy). The ABC add/add_nowait signature is
unchanged, so the summarization flush hook and host are unaffected.
Tests: add test_message_processing_signals, test_updater_truncation,
test_updater_watermark; update queue/updater/consolidation/staleness/pluggable
tests for the signals seam.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(memory): harden update pipeline per PR review
- Catch QueueFull in DeerMem.add/add_nowait so backpressure degrades to
'update skipped' instead of propagating into after_agent /
summarization_hook and breaking the agent run (peer middlewares
self-guard; MemoryMiddleware was the lone exception). Emergency
(add_nowait) always admits under backpressure -- its data cannot be
re-fed next turn.
- Rewrite the watermark from index-based to content/identity-based
(_message_identity + _feed_after_watermark) so it stays correct when
summarization removes the conversation front -- an index watermark
pointed at the wrong message and silently skipped un-extracted tail
turns. The emergency flush bypasses the watermark (bypass_watermark on
ConversationContext, threaded through update_memory) and coexists with
(does not replace) a pending normal update, so a flush cannot drop a
pending update's un-extracted tail.
- Populate facts_accepted / rejected_low_confidence inside _apply_updates
at the real confidence-filter site (passed_threshold) instead of
re-deriving the threshold in _finalize_update -- eliminates metric drift.
- Emit extraction metrics in a finally with an 'attempted' flag so
exception failures (parse error, apply_changes raise after retry) are
observable, not only the happy path.
- Re-detect signals on the post-watermark feed for the extraction hint so
it no longer references turns the LLM cannot see; admission-time signals
still drive backpressure.
- Move the post-batch reschedule inside the queue lock to close a
non-atomic self._timer race with a concurrent add.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(memory): address follow-up review nits (LRU, metric name, docstring)
- Bound the in-memory watermark cache with a configurable LRU
(watermark_max_keys, default 4096, 0=unbounded). A dropped key re-extracts
one batch on that thread's next turn (the documented restart behavior), so
eviction is safe and preserves the content-identity watermark's
front-removal guarantee. Adds _watermark_get/_watermark_set helpers and a
bounded-LRU regression test.
- Rename the extraction metric facts_accepted -> facts_passed_confidence so
the name matches what the >60% rejection-rate warning assumes (a
confidence-gate signal, not a persisted-fact count); drop the stale
"historical semantics" justification. Brand-new callback, one consumer.
- Fix the stale test_message_processing_signals module docstring: the signals
seam is already swapped to frozenset, and a stale stage-numbering prefix is
removed.
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: Claude <noreply@anthropic.com>
|
||
|
Zhengcy05
|
2f60bee388
|
fix: surface length-capped model responses (#4309)
* fix: surface length-capped model responses * fix: avoid the influence of the mid-turn * fix: correcting semantic annotations * fix: add ModelLengthTerminationDetector to compatible providers * fix:delete redundancy code * fix:supplementing log information improves observability * fix: align the document and complete the assertions. * fix: unit test * fix: revert AGENTS.md * fix: unit test * fix: add annotation and skip AIMessage has empty content |
||
|
Aari
|
37c343fe30
|
fix(summarization): summarize with the run model, fall back on summary-provider failure (#4361)
* fix(summarization): own the run model for compaction; bound failure With summarization.model_name: null the summary model resolved to config.models[0] while the executing model is selected per run; when they differ and models[0]'s provider is broken (expired key, quota, outage) compaction silently failed every triggered turn and context grew unbounded until the main provider 400s the run (#3103's shape), even though the run's own model was healthy. Model ownership is now sourced from the builders, not re-derived at runtime: - The lead, subagent, and manual /compact builders each pass the resolved run model into create_summarization_middleware(run_model_name=...). The middleware no longer reads runtime.context / get_config(), which do not carry a custom agent's or a subagent's resolved model, so a custom-agent lead run and a distinct-model subagent now summarize with their own model, not models[0] / the parent's. Runtime re-resolution and the per-name model cache are removed. - model_name: null summarizes with the run's own model; an explicitly configured summary model generates and falls back to the run model on failure. The fallback is built lazily after the primary fails and its construction is guarded, so a broken fallback cannot skip a healthy primary or escape the automatic failure boundary. Failure is bounded and side-effect-safe: - An empty or whitespace-only response is treated as a generation failure, not a valid summary, so compaction never removes all history for an empty replacement. - compact_state/acompact_state take raise_on_failure independent of force: the manual /compact path always surfaces a generation failure (even force=false) and routes it to the existing ContextCompactionFailed path (HTTP 500 -> frontend error toast) instead of an unconsumed response reason. The automatic path leaves compaction state unchanged. - before_summarization hooks fire only after a replacement summary exists. SummarizationConfig.model_name, config.example.yaml, and docs/summarization.md document the final lead/subagent/manual ownership rules. Part of RFC #4346 (section A). Evaluating fraction/triggers against the run model's profile (profile ownership) is a separate follow-up. * fix(summarization): manual /compact model ownership + fail-open construct/parse Manual /compact carried only agent_name, so it derived the run model from the custom-agent model or config.models[0] and missed the request-selected model the run path uses (request -> custom-agent -> default). Carry model_name through ThreadCompactRequest and the frontend compact call, resolve with the same precedence, and move the custom-agent config read off the event loop (asyncio .to_thread) with user_id so the strict blocking-IO gate is not bypassed by the broad except. Make one summary attempt own its full lifecycle so the fail-open boundary covers construction and response parsing, not just invocation: build each candidate model lazily and guarded (a raising constructor falls through to the healthy run model instead of breaking agent construction), build the model_name:null primary from the run model rather than config.models[0], and run response text extraction inside the invocation try so a failing .text accessor falls back instead of escaping compaction. Adds factory-level constructor-failure, response-extraction-failure (sync/async), and route-path model-ownership tests. |
||
|
Vanzeren
|
8c19a2eb36
|
perf(checkpoint): linearize message write merging (#4421)
* perf(checkpoint): linearize message write merging * test(checkpoint): address message reducer review |
||
|
黄云龙
|
126fc9ea81
|
fix(subagents): clamp subagent limit consistently with MIN_SUBAGENT_LIMIT (#4081)
* fix(subagents): align prompt and middleware subagent limit; allow min of 1 SubagentLimitMiddleware clamped max_concurrent to [2, 4] internally, but agent.py and client.py fed the raw config value into the system prompt, so a user-configured 1 (or 5) produced a prompt that disagreed with the enforced middleware limit. Lower MIN_SUBAGENT_LIMIT to 1 and clamp the raw config value with _clamp_subagent_limit() at both the agent factory and the embedded client so the prompt and middleware see the same value. * fix: remove unused imports MAX_CONCURRENT_SUBAGENT_CALLS, MIN_CONCURRENT_SUBAGENT_CALLS, clamp_subagent_concurrency * fix: harmonize clamp range [1,4] across middleware, config, and prompt path; fix lint - Changed MIN_CONCURRENT_SUBAGENT_CALLS from 2 to 1 so prompt.py's clamp_subagent_concurrency and the middleware's _clamp_subagent_limit both clamp to [1,4] — eliminating the divergence where the prompt told the model 'max 2 task calls' but the middleware enforced 1. - Applied _clamp_subagent_limit at build_middlewares (agent.py:360) so all 3 construction sites (agent.py:360, agent.py:450, client.py:259) consistently clamp the config-resolved limit. - Derived MIN_SUBAGENT_LIMIT / MAX_SUBAGENT_LIMIT from MIN_CONCURRENT_SUBAGENT_CALLS / MAX_CONCURRENT_SUBAGENT_CALLS so the two module-level definitions stay in sync. - Added TestConfigParity.test_prompt_path_and_middleware_clamp_agree regression test. - Fixed lint. * fix(lint): add missing imports for MIN_CONCURRENT_SUBAGENT_CALLS and MAX_CONCURRENT_SUBAGENT_CALLS * docs+test: update AGENTS.md clamp range to 1-4; add prompt/middleware parity regression test - backend/AGENTS.md still documented the old [2,4] clamp in two places; updated to [1,4] to match MIN_CONCURRENT_SUBAGENT_CALLS = 1. - Added test_apply_prompt_template_single_subagent_limit_matches_middleware: renders the real system prompt with max_concurrent_subagents=1 and asserts the advertised HARD LIMITS value equals SubagentLimitMiddleware's enforced max_concurrent — the end-to-end check that would have caught the [1,4] vs [2,4] prompt-path divergence flagged in review. * refactor: simplify per review — restore clamp delegation, drop redundant call-site clamps Per willem-bd's review, reduce the PR to the one behavioral change plus docs/tests: - _clamp_subagent_limit delegates to clamp_subagent_concurrency again instead of inlining a byte-identical copy; with a single source of truth the TestConfigParity sync-check class is unnecessary — dropped. - Revert the call-site clamps in agent.py (build_middlewares, _make_lead_agent) and client.py (_ensure_agent) to main: both downstream consumers (SubagentLimitMiddleware.__init__ and the prompt path) already clamp internally, and the cross-module private import of _clamp_subagent_limit goes away with them. - Keep MIN_CONCURRENT_SUBAGENT_CALLS = 1 (the fix), the [1, 4] docstring updates, the AGENTS.md range corrections, and the end-to-end prompt/middleware parity test for single-subagent mode (docstring reworded: on main a configured 1 was bumped to 2 by both paths — there was no divergence to fix, just a silently raised floor). * test: fix stale comment referencing reverted agent.py/client.py call-site clamps --------- Co-authored-by: nankingjing <nankingjing@users.noreply.github.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Huixin615
|
4a2ecd430e
|
fix(streaming): expose custom events to astream_events (#4403)
* fix(streaming): expose custom events to astream_events * test(streaming): validate real custom event emitters --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
hataa
|
7857fa0cce
|
feat(authz): enforce tool authorization at assembly and runtime (#4370)
* feat(authz): enforce tool authorization at assembly and runtime * fix(middleware): guard deferred tool setup lookup (#4370) --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
MiaoRuidx
|
f1632cc351
|
fix(run): add run event stream contract (#4342)
* docs: document run event stream contract * fix(run): address event stream review feedback --------- Co-authored-by: MiaoRuidx <12540796+MiaoRuidx@users.noreply.github.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Aari
|
b7933d18e4
|
fix(safety): backfill empty content-filter responses so they don't poison the thread (#4394)
An empty assistant message from a provider safety filter (content_filter with
no content, no tool calls) was persisted into thread history and replayed to
strict OpenAI-compatible providers, which reject it with HTTP 400 ("message ...
with role 'assistant' must not be empty") — breaking every later turn until a
new chat is started.
SafetyFinishReasonMiddleware only handled the tool-call case (#3028) and
TerminalResponseMiddleware only the post-tool case (#4027), so a plain empty
content-filter response fell through both. Extend the safety middleware to
backfill a user-facing explanation when a safety-terminated message is
otherwise blank, so the persisted turn is non-empty (and the user sees why it
was blocked).
Fixes #4393
|
||
|
lllyfff
|
01a89f2379
|
[feat] memory: pluggable MemoryManager interface for backend onboarding (#4326)
* refactor(memory): pluggable MemoryManager interface for backend onboarding
Optimize the MemoryManager interface layer so new backends (mem0/openviking)
onboard with less code and the contract stays stable as capabilities are
added. A minimal backend now implements only from_config + add + get_context
(verified by test_memory_manager_interface.py::_MinimalBackend onboarding via
the factory); the factory no longer knows a backend's private hooks.
- MemoryManager: ABC -> pydantic BaseModel; three-tier methods (tier-1
add/get_context abstract; tier-2 management defaults; tier-3 optional hooks
warm/reload/fact + on_pre_compress/on_turn_start). Dropped 3 self-serving
hooks. 6 hasattr probe sites -> direct call + try/except NotImplementedError.
- from_config classmethod: factory thins to resolve + inject storage_path +
collect host hooks + call from_config; DeerMem-specific hook consumption
moved from factory to DeerMem.from_config.
- Invariants: @model_validator (mode='tool' requires search via supports_search
ClassVar); DeerMemConfig storage_path-is-file check moved here from factory.
- Async: aadd/aget_context/asearch default to the sync path (speculative).
- Callbacks: MemoryCallbacks + LangfuseMemoryCallbacks; on_memory_llm_call
subsumes tracing_callback (same signature/timing/mutation); deleted the
tracing_callback field. DeerMem decoupled from langfuse (portability).
- noop keeps read-op empty overrides (avoids router 500s on the
disable-memory-via-noop path); only delete/export inherit the base raise.
Behavior preserved: 661 passed / 13 skipped. Docs: backends/README.md rewritten
(three-tier + from_config + callbacks); samples README updated; removed stale
private doc paths.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(memory): 501 on unsupported read/manage endpoints + accurate warm log
Review follow-up on the three-tier MemoryManager refactor.
- Read/manage endpoints (GET /memory, /memory/export, /memory/status,
DELETE /memory, POST /memory/import) and the /memory/reload fallback now
catch NotImplementedError -> 501, matching the fact-CRUD endpoints. The
hasattr->try/except migration had skipped these: they were @abstractmethod
before (every backend implemented them, so they never raised), so once they
became tier-2 default-raise a minimal backend (only add + get_context) hit a
raw 500 -- there is no global NotImplementedError handler. get_memory is
shared via _get_memory_or_501 (covers /memory, export, status, reload
fallback). noop is unchanged: its read-op empty overrides never raise.
- warm() base default returns None (tri-state: True=warmed, False=failed,
None=nothing to warm) so the Gateway lifespan logs "skipping" for a
non-DeerMem backend (e.g. noop) instead of the inaccurate "warmed
successfully" it never earned. DeerMem.warm keeps True/False.
- Tests: 6 router 501 tests (read/manage + reload fallback) + 2 lifespan
warm-log tests (None->skipping, False->warning); conformance/pluggable
assert warm() is None.
705 passed / 13 skipped; lint clean.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(memory): review follow-ups - search-flag consistency, client reload, backend_config purity
Address review feedback on the three-tier MemoryManager refactor:
- [Medium] supports_search/search drift: the invariant now requires the
supports_search ClassVar flag to MATCH whether search() is actually
overridden (type(self).search is not MemoryManager.search), so the flag
can't drift from the impl. Catches both directions at instantiation: a
backend that overrides search() but forgets supports_search=True (was a
misleading tool-mode rejection), and one that sets the flag without
overriding (was a runtime NotImplementedError on the first memory_search).
noop sets supports_search=True to match its search() override. Conformance
adds drift + consistent-backend tests.
- [Low] client.reload_memory fallback: wrap the get_memory fallback so a
minimal backend (only add + get_context) surfaces a clean NotImplementedError
("implements neither reload_memory nor get_memory") instead of an uncaught
propagation -- mirrors the router's 501. Test added.
- [Low] backend_config purity: DeerMem.from_config restores backend_config to
the pure data the host passed after model_post_init parses the injected hooks
into DeerMemConfig (self._config, PrivateAttr); the field stays serializable
(no callables/LLM) and matches the README ("host hooks NOT in backend_config").
Test asserts purity + hooks wired.
- [Low] CHANGELOG: breaking-change note that mode='tool' + non-search backend
now fails fast at startup (was silently empty) so operators recognize it on
upgrade.
- [Nit] .gitignore: drop the env-specific .tmp-pytest/ entry (--basetemp is
local-only, not make test/CI).
709 passed / 13 skipped; lint clean.
Co-Authored-By: Claude <noreply@anthropic.com>
* docs(changelog): correct memory tool-mode fail-fast note
The CHANGELOG entry said mode='tool' + a non-search backend "(e.g. noop)"
fails fast at startup, but noop overrides search() (returns []) and sets
supports_search=True (required by the consistency invariant), so noop IS
search-capable and noop+tool does NOT fail fast. The fail-fast only affects a
custom backend that onboards without overriding search(). Reworded to drop the
misleading noop example and state both shipping backends implement search().
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: Claude <noreply@anthropic.com>
|
||
|
Lee minjing
|
e225ad57d7
|
feat(uploads): lazy-load historical files via list_uploaded_files tool (#4174)
* feat(uploads): lazy-load historical files via list_uploaded_files tool
Replace per-turn injection of all historical upload metadata with on-demand
discovery via a new `list_uploaded_files` built-in tool, following the same
deferred-discovery pattern used by skills.
- Rename <uploaded_files> block to <current_uploads> (current-run files only)
- Add list_uploaded_files tool with include_outline: bool|list[str]
- Extract outline helpers to shared deerflow/utils/file_outline.py
- Update system prompt to reflect lazy-loading behaviour
- Historical file scan removed from UploadsMiddleware.before_agent()
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(uploads): clear uploaded_files state when no new files in current turn
When before_agent() returns None on empty turns, the LastValue
uploaded_files field retains the previous turn's filenames.
list_uploaded_files then incorrectly excludes those files as
"current-run" files, making them invisible until the next upload.
Fix: return {"uploaded_files": []} instead of None to explicitly
clear state. Add two-turn regression test covering the exact
scenario from review feedback.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix: resolve CI lint errors and stale test assertion from merge
- Split long prompt line to fit 240-char limit
- Add missing `Any` import in list_uploaded_files_tool
- Remove unused `re` import in file_conversion (outline code moved)
- Remove unused `os` import in middleware test
- Fix test assertion: <uploaded_files> → <current_uploads> after main merge
Co-Authored-By: Claude <noreply@anthropic.com>
* fix: resolve CI lint errors and stale test assertion from merge
- Split long prompt line to fit 240-char limit
- Add missing `Any` import in list_uploaded_files_tool
- Remove unused `re` import in file_conversion (outline code moved)
- Remove unused `os` import in middleware test
- Fix test assertion: <uploaded_files> → <current_uploads> after main merge
Co-Authored-By: Claude <noreply@anthropic.com>
* fix: add current_uploads to input sanitization exempt tags
The lazy-loading PR renamed <uploaded_files> to <current_uploads>.
The anti-drift guard scans all framework XML blocks and requires each
to be either blocked or explicitly exempted. current_uploads wraps
trusted server-generated file metadata, not user input, so it belongs
in the exempt set.
Co-Authored-By: Claude <noreply@anthropic.com>
* test: regenerate replay golden after uploaded_files state change
before_agent now returns {"uploaded_files": []} instead of None,
adding uploaded_files to SSE values events. Regenerated via
DEERFLOW_WRITE_GOLDEN=1.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix: review feedback — memory pipeline, stale tags, state clearing, nits
- Match both tags in memory stripping pipeline (uploaded_files|current_uploads)
- Remove stale uploaded_files from _BLOCKED_TAG_NAMES
- Clear uploaded_files on all before_agent early-return paths
- Fix ponytail: stray word in file_conversion re-export comment
- Remove dead total_omitted branch in _format_omitted_summary
- ruff format fixes
Co-Authored-By: Claude <noreply@anthropic.com>
* fix: block current_uploads, sanitize only original user content
Per review feedback: instead of exempting <current_uploads> (which
allows user forgery), move it to _BLOCKED_TAG_NAMES and change
InputSanitizationMiddleware._process_request to scan only the
original user content (ORIGINAL_USER_CONTENT_KEY) when available.
Server-injected trusted blocks are no longer checked against the
blocked-tag denylist.
Co-Authored-By: Claude <noreply@anthropic.com>
* docs: clarify fallback reason in input sanitization comment
Co-Authored-By: Claude <noreply@anthropic.com>
* @
fix: third-round review feedback — state visibility, sanitization, regex, nits
- list_uploaded_files_tool: logger.warning instead of silent try/except
on runtime.state read failure (High)
- input_sanitization_middleware: _extract_text_from_content skips empty
text blocks to match message_content_to_text behaviour; rfind fallback
path logs warning for observability (Medium)
- memory pipeline regexes: backreference (?P<tag>)(?P=tag) in
message_processing.py and prompt.py (Low)
- file_conversion.py: re-export moved to top of file (Low)
- Tests: middleware→tool state bridge test; integrated forged-tag +
multimodal sanitization tests
PR #4174 — Follow-up issues: #4212, #4213, #4214
Co-Authored-By: Claude <noreply@anthropic.com>
@
* @
fix: 4th-round review — denylist, sanitization, scandir, nits
- Add "uploaded_files" back to _BLOCKED_TAG_NAMES (old tag still processed by
deermem; user forgery must be escaped) (consistency)
- Fix inaccurate rfind-fallback comment: UploadsMiddleware keeps string as
string, fallback is unreachable for strings (doc fix)
- Distinguish "empty string key" (upload without text) from "non-string key"
(caller forgery) so empty-text uploads never escape the server block (edge)
- Merge dual os.scandir(uploads_dir) calls into one list re-use (minor)
- Add comment on .md sibling skip known limitation: user-uploaded .md files
whose stem collides with a converted doc are hidden (boundary, no code change)
Co-Authored-By: Claude <noreply@anthropic.com>
@
* @
fix: tighten rfind-failure fallback — distinguish server blocks from user blocks
When _extract_text_from_content and message_content_to_text disagree on
multimodal list content and rfind fails, use content[0] (server-injected
<current_uploads> block) vs content[1:] (user blocks) to sanitize only
user blocks. Raw strings and non-standard dict blocks that
_extract_text_from_content misses are now also sanitized.
Non-distinguishable paths (< 2 text blocks, non-list content) still
degrade to full sanitization (safe — server block may be escaped but
user forgery never leaks). All fallback paths log via logger.warning.
Decision 18 / willem-bd 4th-round comment #3
Co-Authored-By: Claude <noreply@anthropic.com>
@
* @
fix: correct comments referencing text_blocks → content in rfind fallback
Co-Authored-By: Claude <noreply@anthropic.com>
@
* fix: 5th-round review — dead code, subagent gating, integration test, perf, consistency
- Delete unreachable ORIGINAL_USER_CONTENT_KEY guard in rfind fallback
branch (original_user_content guaranteed non-empty str at that point)
- Remove list_uploaded_files from BUILTIN_TOOLS; add include_upload_tool
param to get_available_tools(), default True; task_tool.py passes False
so subagents no longer receive a tool whose state exclusion is broken
- Add integration test exercising real create_agent graph (not mocked
runtime.state) to verify LangGraph propagates before_agent state writes
into ToolRuntime.state during same-turn tool calls
- Cache DirEntry.stat() st_size in candidates tuple to avoid second
per-file syscall in the rendering loop
- Make the upload-tag pre-check case-insensitive (content_str.lower())
to match _UPLOAD_BLOCK_RE re.IGNORECASE
PR #4174 — willem-bd 5th-round review items #1-#5
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(channels): pass files metadata through _human_input_message() for IM uploads
_human_input_message() was not passing additional_kwargs.files to the
downstream message. UploadsMiddleware read no files, wrote
uploaded_files=[], and list_uploaded_files reported same-run IM
attachments as historical files (fancyboi999 repro).
Fix: add files parameter to _human_input_message(), call site passes
files=uploaded. Regression test locks the contract.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(channels): remove legacy <uploaded_files> manual prepend to fix double-injection regression
Commit 8d86dbf6 added files= pass-through to UploadsMiddleware but
left the manual _format_uploaded_files_block() prepend in place.
Every IM attachment reached the model twice — once via the legacy
<uploaded_files> block and once via <current_uploads>.
This commit removes the manual prepend and the now-dead
_format_uploaded_files_block() function. UploadsMiddleware is the
sole upload-context producer for both IM and web paths.
Reported-by: fancyboi999 (PR review)
Co-Authored-By: Claude <noreply@anthropic.com>
* docs: update #4212 issue body to reflect completed fixes and narrowed remaining scope
* chore: remove temporary scratch file
* fix(middleware): neutralize user-derived values inside <current_uploads> block
Upload-derived filenames, paths, outline titles, and preview text are
interpolated verbatim inside the trusted <current_uploads> wrapper,
which InputSanitizationMiddleware exempts from sanitization. A crafted
filename or document heading containing blocked authority tags would
bypass the guardrail and enter model context as trusted framework data.
Fix: call neutralize_untrusted_tags() on all four user-derived values
inside _format_file_entry(), preserving the outer <current_uploads>
wrapper untouched.
Reported-by: fancyboi999 (P1 security review)
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(middleware): neutralize extension labels in omitted-file summary
Files exceeding the 10-item context cap bypass _format_file_entry().
Their extensions, derived from user-controlled filenames via
_extension_label(), were interpolated verbatim into the trusted
<current_uploads> wrapper — another path for blocked authority tags
to escape the guardrail.
Fix: neutralize extension values inside _extension_label(), the
single extraction point for all extension labels.
Reported-by: fancyboi999 (P1 security review)
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(tools): neutralize user-derived values in list_uploaded_files tool result
Apply neutralize_untrusted_tags() to every model-visible user-derived value
returned by list_uploaded_files: filename, virtual path, extension, outline
titles, outline preview lines, and omitted-file extension summary.
This closes the last remaining injection bypass in the upload lazy-loading
path - the <current_uploads> block and its omitted summary were already
neutralized (previous commits), but the list_uploaded_files tool produced
a second exit for the same attacker-controlled metadata that
ToolResultSanitizationMiddleware did not cover.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(tests): add missing include_upload_tool=False to task_tool mock assertions
PR #4174 added include_upload_tool parameter to get_available_tools().
task_tool.py correctly passes include_upload_tool=False for subagents
but 5 existing tests' assert_called_once_with expectations were not
updated, causing CI failures.
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: Claude <noreply@anthropic.com>
|
||
|
Ryker_Feng
|
20debf9cc7
|
feat(agents): per-agent model and generation settings (#4347)
* feat(agents): per-agent model and generation settings Let each custom agent choose its own model and sampling settings (temperature, max_tokens) plus thinking / reasoning_effort defaults, so agents sharing a model profile are no longer stuck with one shared temperature and output length (#4336). AgentConfig gains optional model_settings / thinking_enabled / reasoning_effort (None = inherit). create_chat_model applies per-caller model_overrides on top of the profile before the thinking/Codex transforms; the lead agent resolves each knob with precedence request > agent config > profile/default. The /api/agents create/update routes persist the fields and reject an unknown model. The default lead agent path is unchanged (no agent config -> overrides None). The agent chat composer also stops force-overriding an agent's configured default model with models[0]. * fix(agents): tri-state thinking control and default-model capability gating The model-settings dialog seeded the thinking switch to false, so opening it to tweak temperature and saving silently disabled thinking (the runtime default is on) with no way back to inherit. It also hid the thinking / reasoning controls whenever the agent inherited the global default model, since `__default__` never resolved through `models.find`. Give thinking an explicit Inherit / On / Off tri-state so an untouched save is a no-op, and resolve `__default__` to the effective default (models[0]) for the capability check. Logic lives in the tested helpers module. |
||
|
March7
|
ce4a6d4c3d
|
fix(backend): remove transient image context after model calls (#4267)
* fix(backend): discard transient image context * fix(backend): protect client image context ids * docs(backend): clarify image checkpoint lifecycle |
||
|
Vanzeren
|
42baed8c8c
|
feat(checkpoint): dual-mode checkpoint storage with LangGraph DeltaChannel (#4292)
* feat(checkpoint): dual-mode checkpoint storage with LangGraph DeltaChannel
Add a restart-required database.checkpoint_channel_mode ("full" default,
"delta") that stores the messages channel via LangGraph 1.2 DeltaChannel,
cutting checkpoint storage from O(n^2) to O(n) for append-only history.
Existing full checkpoints seed delta state transparently; no data migration.
- config: mode schema + freeze-on-first-use with
CheckpointModeReconfigurationError; mode marker persisted in checkpoint
metadata; unsafe delta->full downgrade rejected fail-closed with
CheckpointModeMismatchError (run-level error, failed state read)
- state: delta message state schema; CheckpointStateAccessor centralizes
materialized reads for all consumers (threads API, branches,
regeneration, compaction, state updates, memory, goal workers)
- runtime: raw writers (run durations, interrupted title, thread goal)
parent their checkpoints to the checkpoint they derive from, preserving
delta ancestry; rollback forks the pre-run lineage through a state
mutation graph with Overwrite restores; InMemorySaver delta-history
override delegates to the base walk (fixes dropped first write after
migration, also present upstream)
- tests: conformance suite over {memory, sqlite, postgres} covering
migration replay, stable message IDs, storage shape and writer
preservation; conftest fixture isolates the frozen mode between tests;
stale config fakes refreshed
- ci: backend unit tests gain a postgres service
* fix(checkpoint): close materialization gaps in goal flow, guard public factory
- Route goal-continuation message reads through CheckpointStateAccessor:
raw channel_values reads see the delta sentinel in delta mode, which
disabled goal continuation (stand_down=no_durable_end_of_turn) after
durable assistant turns. Raw tuples remain for tuple-only metadata
(checkpoint id, pending_writes).
- Reject checkpoint_channel_mode='delta' + checkpointer in
create_deerflow_agent at construction: factory-built persisted graphs
bypass mode-marker injection and the fail-closed gate, reproducing
silent mixed-mode state loss. Delta without persistence stays allowed.
- Import the postgres saver lazily (pytest.importorskip in the fixture)
so the documented default install collects the suite; add a CI job
running pytest --collect-only on uv sync --group dev without extras.
- Fix test_checkpointer fallback test to patch get_app_config at its
use site (provider module), making it deterministic when a local
config.yaml selects a persistent backend.
* fix(gateway): preserve extension-owned channels in state mutations, bump config version
- build_state_mutation_graph / build_checkpoint_state_mutation_accessor
accept an explicit state_schema; branch and POST /state now compile the
mutation graph from the thread's effective schema
(graph_state_schema on the assistant graph). The base-ThreadState
fallback silently discarded channels contributed by custom
AgentMiddleware.state_schema on branch (data loss) and returned a
false-success 200 on POST /state.
- POST /state validates values keys against the mutation graph's
channels and rejects unknown fields with 422 instead of ignoring
them; reducer detection covers extension channels
(BinaryOperatorAggregate or DeltaChannel) so Overwrite replace
semantics work for middleware reducers in both modes.
- Endpoint regression: custom AgentMiddleware.state_schema value
survives branch, updates through POST /state, and an unknown field
receives 422.
- config_version 26 -> 27 for the new database.checkpoint_channel_mode
(example, Helm chart values + README, support-bundle fixture), so
existing installs get the outdated-config warning and
make config-upgrade merges the field; covered by a test driving the
real example file and the real config-upgrade script.
* fix(gateway): resolve assistant schema via one boundary, copy branch reducer values with Overwrite
GET /threads/{id}/state now resolves the thread's assistant_id through a
single reusable boundary (thread metadata -> assistant_id -> effective
graph), so channels contributed by a custom AgentMiddleware.state_schema
are materialized instead of dropped by the default lead schema. POST
/state uses the same boundary instead of resolving the schema ad hoc.
Branch writes wrap every copied reducer channel in Overwrite (derived
from the effective mutation graph: BinaryOperatorAggregate + DeltaChannel),
not just messages, so already-aggregated values are never re-merged.
Regression tests use a real AgentMiddleware.state_schema with a
non-identity reducer in both full and delta modes: GET /state returns the
extension value, POST /state replaces it, branch preserves it
byte-for-byte; the unknown-field 422 is a separate assertion.
* refactor(checkpoint): collapse read-path round-trips and ship dual-mode parity tests
Address review round 4 on PR #4292:
- Push ahistory/history limit through Pregel into checkpointer.alist
(SQL LIMIT) instead of materializing all rows and breaking in Python
- Fold the read-side mode-compat gate onto the returned snapshot's
metadata; only writes keep the pre-write tuple fetch (fail-closed)
- Cache factory-built accessor graphs per (assistant_id, mode) with
factory-identity revalidation; state reads no longer build a lead
agent per request
- get_thread: one snapshot fetch + one raw pending_writes fetch on the
resolved checkpoint (post-checkpoint __error__ writes never surface
in snapshot.tasks; verified empirically)
- DeerFlowClient.get_thread: single checkpointer.list walk collects
pending_writes per checkpoint instead of N get_tuple calls
- InMemorySaver delta-history patch: stand-down when the upstream
override disappears, try/except guard, validated-version warning,
guard tests
- make_lead_agent mode precedence: first freeze is owned by app_config
(client-supplied configurable key ignored); once frozen, injected
key/app_config must match or fail closed
- Rollback: lock in non-message channel restoration via fork
inheritance with a dedicated reducer-channel test
- Add tests/test_threads_checkpoint_mode.py and
tests/test_gateway_checkpoint_mode.py referenced by AGENTS.md and
the PR validation section: lifecycle parity (memory + sqlite),
per-step blob-count storage guard, gateway endpoint parity
Counted-saver tests pin checkpoint round-trips for aget/ahistory so
these regressions cannot silently return.
* fix(checkpoint): precise mode-mismatch HTTP mapping, gate E2E, and accessor resilience
- threads router: map CheckpointModeMismatchError to 409 (with cause and
thread id) and CheckpointModeReconfigurationError to 503 across all state
endpoints instead of swallowing both into a generic 500
- gate coverage: seed a real delta checkpoint into AsyncSqliteSaver and
assert aget/aupdate/ahistory fail closed; assert 409 at the HTTP boundary
through the real route stack
- rollback: compile the restore mutation graph with the thread's effective
state schema per the build_state_mutation_graph contract
- inheritance contract locks: rollback and manual compaction preserve
middleware-contributed channels via checkpoint fork cloning
- services: revalidate the accessor-graph cache against app_config identity
so config.yaml hot-reloads never serve a stale compiled graph
- services: degrade full-mode state reads to raw checkpointer reads when the
agent factory is unavailable (delta gate still applies; delta mode has no
fallback)
- deps: override websockets==16.0 (langgraph-sdk 0.4.2's <16 pin silently
downgraded 16.0 -> 15.0.1; pin is not grounded in any API incompatibility)
and bump the langchain lower bound to what the lockfile actually resolves
* fix(checkpoint): include anchor checkpoint in degraded history walk + cover get_thread
- _RawCheckpointReadAccessor.ahistory: alist(before=...) is exclusive while
pregel's get_state_history treats config.checkpoint_id as the inclusive
start; fetch the anchor explicitly so both read paths paginate identically
- extend the degraded-path gateway test: GET /thread returns raw values, and
POST /history with before starts at the anchor checkpoint
* fix(gateway): preserve degraded checkpoint timestamps
* fix(gateway): harden degraded checkpoint access
* fix(gateway): resolve assistants for checkpoint reads
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
|
||
|
RongfuShuiping
|
4bf028d048
|
feat(memory): add incremental agent-scoped Markdown fact storage (#4279)
* feat(memory): add Markdown fact storage repository * docs(memory): explain storage rewrite for beginners * docs(memory): fix plan markdown formatting * refactor(memory): separate global summaries from agent facts * fix(memory): make Markdown fact updates incremental and safe * Update STORAGE_REWRITE_CHANGES.md * Delete docs/plans/STORAGE_REWRITE_PLAN.md * Delete docs/plans/STORAGE_REWRITE_CHANGES.md * fix(memory): address Markdown storage review feedback * fix(memory): complete review follow-ups * fix(memory): resolve storage review findings * feat(memory): add proactive Markdown migration CLI * fix(memory): harden Markdown storage concurrency * fix(memory): harden markdown storage migration * fix(memory): close migration review gaps --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> Co-authored-by: qin-chenghan <qinchenghan@Huawei.com> |
||
|
ChenglongZ
|
ca16b64b26
|
feat(agent): support config-declared lead middlewares (#3964)
* feat(agent): support config-declared lead middlewares * fix(agent): preserve configured extension middlewares * fix(agent): address middleware extension review * fix(agent): tighten middleware extension docs and tests |
||
|
Tianye Song
|
8511fa6aa3
|
fix(memory): consolidated facts inherit expected_valid_days from sources (#4225)
* fix(memory): consolidated facts inherit expected_valid_days from sources Consolidation (#3996) and per-fact expected_valid_days (#4143) were both authored by the same contributor but never connected: the consolidated new_fact carried the newest source's createdAt but no expected_valid_days, so _effective_fact_staleness_age fell back to the global staleness_age_days. A merge of several 200-day-old stable facts (each evd=3650) would land with no evd, read as a 90-day window, and re-enter the staleness candidate set on the very next cycle - the merge discarded the lifetime signal of the underlying information and contradicted consolidation's premise (these are stable, related facts worth synthesising). Fix: the merged fact inherits expected_valid_days set so it is re-reviewed at the EARLIEST source review deadline (min(createdAt + expected_valid_days) across sources, relative to the merged fact's createdAt = the newest source's). A merge combines details from every source, so a volatile sub-detail (evd=7) must not inherit a stable source's 3650-day window and escape staleness review for years - staleness KEEP/REMOVE is the only path that re-validates a merged fact, so biasing toward the soonest deadline keeps uncertain merges re-checked sooner. A source already past its deadline yields a minimal positive window (review next cycle) rather than the global fallback, which would defer an overdue review. Capped at the creation-time staleness_max_lifetime_multiplier like any new fact. Omitted when no source carries a valid evd (legacy facts fall back to the global age at read time, matching pre-feature behaviour). DRY: extract _read_expected_valid_days(fact) -> int | None, the shared type rule (int/float, reject bool, coerce to int BEFORE the > 0 guard) previously inlined in four places - _normalize_memory_update_fact, _effective_fact_staleness_age, the newFact creation cap, and consolidation inheritance. All four call the single helper. Coercing before the guard matters for values in (0, 1): 0.5 passes a raw > 0 check but truncates to 0, which would violate the helper's "positive int or None" contract; the order now matches the original _normalize_memory_update_fact rule. No prompt/schema change: consolidation's prompt does not surface source evd to the LLM, so asking the model to assign a merged lifetime would be guessing without signal. Source inheritance is deterministic and always available. Tests: consolidation evd cases now use time-stable createdAt (relative to now via a _days_ago helper) covering - earliest-deadline selection, creation-cap clamp, omit when no source evd, volatile source governs the deadline (and re-enters staleness next cycle), overdue source clamps to a minimal window, float coercion. Plus TestReadExpectedValidDays / TestEffectiveFactStalenessAge regression cases for the (0, 1) coercion-order fix. * fix(memory): reject non-finite expected_valid_days before int coercion The shared _read_expected_valid_days helper (introduced when consolidating the evd type rule across four call sites) coerces with int(raw) before the > 0 guard - reversing the original _normalize_memory_update_fact order so that a fractional 0.5 does not leak as 0. But int(raw) raises for non-finite floats: int(nan) raises ValueError and int(inf)/int(-inf) raise OverflowError. Python's JSON decoder accepts NaN / Infinity as floats by default, so a single malformed expected_valid_days in a hand-edited memory.json would abort staleness selection or consolidation instead of falling back to the global lifetime. On main, _effective_fact_staleness_age checked raw > 0 first, so NaN fell back safely (nan > 0 is false) - but inf did NOT (inf > 0 is true, so main also crashed on inf). This helper is now the persisted-fact read path for both staleness and consolidation, so the regression (and the pre-existing inf crash) must be closed here. Fix: require math.isfinite(float(raw)) before the int() coercion, then keep the existing positivity check and fallback. NaN / +/-inf all return None, so callers fall back to the global staleness_age_days. Normal int/float values (including large ones) are unaffected - isfinite is a no-op for them. Tests: - TestReadExpectedValidDays.test_rejects_non_finite_values - NaN, inf, -inf return None (not raise). - TestEffectiveFactStalenessAge.test_falls_back_for_non_finite_values - the persisted-fact read path returns the global age for each, no raise. - test_consolidation_with_non_finite_source_evd_does_not_raise - end-to-end: a NaN-evd source merged with a stable source does not abort consolidation; the NaN source's effective lifetime falls back to the global 90 and its deadline participates in the earliest-deadline computation. * test(memory): hoist _select_stale_candidates import + tidy deadline docstring Two review nits from the latest pass: - `_select_stale_candidates` was imported inline inside three test methods; hoisted to the module-level import block so the dependency is declared once. - `test_consolidated_evd_volatile_source_with_equal_created_at_future_deadline` had an abandoned calculation in its comment ("3 + 7 = 10 ... minus 3 already elapsed = 7? No:") that could mislead future readers into thinking elapsed-since-creation factors into the inherited window. Collapsed to a single clear line stating the window is relative to the merged createdAt, regardless of the source's current age. * fix(memory): reject huge-int expected_valid_days above timedelta.max.days _read_expected_valid_days routed every numeric value through float(raw) for the math.isfinite guard, but Python's JSON decoder parses an integer literal with no decimal point as an arbitrary-precision int (unlike 1e400, which decodes to float inf). So a hand-edited memory.json carrying "expected_valid_days": 10**400 raised OverflowError in float(raw) before math.isfinite was ever called - exactly the malformed-field-aborts-everything scenario the helper's docstring claims to prevent. The earlier non-finite fix only closed the float cases (NaN / +/-inf / 1e400). A huge int below the float limit but above timedelta.max.days (e.g. 10**12) would pass the helper and raise OverflowError downstream in timedelta(days=evd) during staleness selection or consolidation - the same crash fancyboi999 flagged for extend_by_days, just reached via a stored evd. Fix: branch on type so an int never passes through float() (matching the reviewer's suggestion), AND cap the returned int at timedelta.max.days (999999999) so the downstream timedelta(days=evd) call cannot overflow either. The float branch keeps the isfinite + int() coercion. Both branches share the 0 < evd <= timedelta.max.days positivity/range check. Normal values are unaffected - any legitimate expected_valid_days is far below the cap (the config ceiling staleness_max_extension_days tops out at 36500). Tests (all three layers): - TestReadExpectedValidDays.test_rejects_huge_int_above_timedelta_max - 10**400, 10**12, 10**9, timedelta.max.days+1 return None; timedelta.max.days itself is accepted. - TestEffectiveFactStalenessAge.test_falls_back_for_huge_int_above_timedelta_max - the persisted-fact read path returns the global age, no raise. - test_consolidation_with_huge_int_source_evd_does_not_raise[1e400|1e12|1e9] - parametrized end-to-end: a huge-int-evd source merged with a stable source does not abort consolidation; the bad source falls back to the global 90. * fix(memory): guard datetime arithmetic, not just timedelta construction The huge-int fix capped _read_expected_valid_days at timedelta.max.days, but that only proves timedelta(days=evd) can be constructed - adding it to a real fact timestamp still overflows datetime.max. @fancyboi999 reproduced it: a source with expected_valid_days=timedelta.max.days raises "OverflowError: date value out of range" at dt + timedelta(...) in the new consolidation deadline calculation. capping at timedelta.max.days was another patch chasing the next overflow boundary, not a real close. Root cause: the helper was doing datetime-range validation, but the safe bound depends on the datetime the evd is added to, not on the evd alone. So the responsibility moves to the arithmetic site, with try/except as the terminal guard - no concrete upper bound to be wrong about. Changes: - _read_expected_valid_days returns any positive int (huge ints included, not routed through float). Its job is type/positivity validation only. - New _safe_add_days(dt, days) -> datetime | None wraps dt + timedelta(days), returning None on OverflowError/ValueError. This is the terminal guard - there is no further boundary to overflow because try/except catches any datetime-range failure regardless of magnitude. - _select_stale_candidates uses _safe_add_days(now, -effective_age); a None result means the window is unrepresentably large, so the fact cannot yet be stale and is skipped (not selected). - Consolidation computes each source's deadline via _safe_add_days; a source whose deadline overflows falls back to the global staleness_age_days deadline (same treatment as a legacy no-evd source), so one malformed field cannot abort the merge. Normal values are unaffected - any legitimate expected_valid_days is far below the overflow boundary (the config ceiling staleness_max_extension_days tops out at 36500). Tests: - TestSafeAddDays: normal/negative shifts; 10**400/10**12/10**9 return None; timedelta.max.days (the exact reproduced value) returns None, not raises. - TestSelectStaleCandidates.test_huge_evd_does_not_abort_selection: a fact with a huge evd is skipped, not selected, and selection does not raise. - test_consolidation_with_huge_int_source_evd_does_not_raise now parametrized over [1e400, 1e12, 1e9, timedelta.max.days] - the last is the value that constructs a valid timedelta but overflows datetime arithmetic. - Helper/read-path tests updated to assert huge ints are returned as-is (the overflow guard is no longer in the helper). |
||
|
Willem Jiang
|
ac5fd46281
|
feat(backend): bound LLM call concurrency and shed burst-rate (429) retries (#4294)
* Create a feature of Process-global LLM concurrency cap * Added configuration of llm_call of max_concurrent_calls * Classify limit_burst_rate and expose retry params via config.yaml * refactor(middleware): encapsulate LLM concurrency state in a dataclass Address PR #4294 review feedback (github-code-quality bot): the bare module-level globals _GLOBAL_CONCURRENCY_LOOP / _GLOBAL_CONCURRENCY_LIMIT were flagged as unused - a false positive, since both are read on the recreate condition, but the `global`-declaration pattern tripped the analyzer. Replace the three globals + `global` declaration with a single _ConcurrencyState dataclass singleton mutated in place. Behavior is unchanged (lazy recreate when the running loop or configured limit changes); the state is now co-located and no longer relies on bare globals. dataclasses is already an established harness convention. Co-Authored-By: Claude <noreply@anthropic.com> * fix(middleware): make LLM concurrency limiter process-wide + jitter burst retry Addresses PR #4294 review (fancyboi999, CHANGES_REQUESTED) - two P1 issues. P1 #1: the asyncio.Semaphore limiter was loop-bound, so it recreated per event loop and the cap was NOT process-wide: lead-agent calls (main loop) and subagent calls (the isolated persistent loop in subagents/executor.py) each got their own semaphore, and the sync graph path (wrap_model_call) bypassed the cap entirely. Recreating on loop/limit change also abandoned permits held by the prior instance. Replace it with a _ProcessWideLimiter built on threading primitives (not loop-bound): one limiter shared across every event loop and both sync/async wrappers. The cap is mutable via set_limit (never recreates, so in-flight permits are never abandoned); permits release in finally and async waiters unregister on cancellation, so cancellation never leaks capacity. Wire it into wrap_model_call (sync) too - previously a direct handler() call. P1 #2: the first (and only) burst-rate retry was deterministic at 5000ms. prev_delay_ms was seeded from the 1000ms normal base, so for burst the window collapsed to randint(5000, max(5000, 1000*3)) = randint(5000, 5000) - a fleet that failed together realigned on the same 5s tick. Seed the first retry from the reason-specific base (prev_delay_ms=None on loop init) so the burst window is [burst_base, cap] = [5000, 8000], non-degenerate. Retry-After is still honored verbatim. Tests: rename semaphore tests -> limiter; add an autouse fixture resetting the process singleton; add regressions the reviewer asked for - cross-loop (lead + isolated-loop subagent), two concurrent sync calls, limit-change while a permit is held (same instance, permit preserved), cancellation no-leak, and burst first-retry non-degeneracy with default config (real and seeded RNG) plus a concurrent de-synchronization case. Verified the burst guard goes red on the old logic ({5000}) and green on the new. Co-Authored-By: Claude <noreply@anthropic.com> * Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> * Potential fix for pull request finding 'Statement has no effect' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> * fix(middleware): lossless limiter handoff, generation-aware cap, burst-rate CB gate Address the open P1/P2 review findings on #4294: - P1 #1 (cancellation handoff): reserve the permit for a specific waiter at dequeue time (grant-at-dequeue, _AsyncWaiter.granted) so a waiter cancelled in the post-dequeue / pre-reacquire window hands its reservation to the next waiter (_handoff_granted_permit_locked) instead of stranding it. No cancellation window remains. - P1 #2 (hot-reload generation): move cap updates out of the per-attempt path; give the limiter one generation-aware owner (set_limit_if_newer with a monotonic instance seq proxy for config freshness) so a stale in-flight run cannot rewrite a freshly-lowered cap. max_concurrent_calls is now genuinely hot-reloadable, resolving the reload-boundary inconsistency by option (b) - no STARTUP_ONLY_FIELDS change (retry params truly hot-reload). - P2 (circuit breaker): gate _record_failure on reason != "burst_rate" so burst-rate (limit_burst_rate) exhaustion - a transient slope-throttle, not "provider down" - does not trip the CB and fast-fail ALL calls. - P3: clamp the jitter window to the cap before drawing (uniform spread instead of piling at the cap); document the per-process / GATEWAY_WORKERS cap semantics in config + the field description. Tests: add the reviewer-requested regressions (cancel-after-dequeue handoff; stale-instance-doesn't-overwrite-lowered-cap across sync + isolated-loop async; burst_rate-exhaustion-doesn't-trip-CB sync + async). Each is red on the prior buggy logic and green on the fix. _build_middleware now routes llm_call knobs through AppConfig so __init__ applies the cap. 71 middleware tests pass; 212 across the blast radius (1 pre-existing skip). Co-Authored-By: Claude <noreply@anthropic.com> * fix(middleware): startup-only LLM concurrency cap; report effective retry budget Addresses review feedback on #4294 (fancyboi999 CHANGES_REQUESTED on acfc7617): P1 - the generation guard measured construction order, not config freshness, so a stale AppConfig(cap=3) constructed after a fresher AppConfig(cap=1) could restore the higher cap; and on a downscale 3->1 release() handed excess permits to queued waiters, keeping in_flight pegged at the old cap. Replace the pseudo-generation path with a startup-only cap: the first middleware __init__ resolves and freezes the cap; later instances (newer or older config) are no-ops. No runtime cap mutation means no downscale race and no freshness/construction-order race. Per-call gate is now `limiter is None` only, so a reloaded instance with max_concurrent_calls=0 cannot silently drop the frozen cap. Removes _owner_seq / set_limit_if_newer / _grant_to_queued_locked / _next_instance_seq; file 946 -> 926 lines. P2 - burst-rate calls are capped at 2 attempts but the retry log line, the llm_retry stream event max_attempts, and the user-facing message still used self.retry_max_attempts (3), so the frontend showed 1/3 then stopped after attempt 2. Thread the effective max_attempts (_max_attempts_for) through the logger, _emit_retry_event, and _build_retry_message. Also: document max_concurrent_calls as startup-only in the config field description and config.example.yaml (prose only - the startup-only: prefix is top-level AppConfig-field granularity and would mislabel the otherwise hot-reloadable llm_call section / break the reload_boundary drift test). Rewrite the cap-mutation tests for startup-only semantics; add P2 retry-budget event tests (sync+async, teeth-verified red on the bug); fix bot nits (empty except blocks -> gather(return_exceptions=True); bare await statements -> assigned+asserted). Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> |
||
|
Aari
|
1a1c5def0d
|
fix(agents): classify web_fetch error pages as errors, not successful evidence (#4314)
* fix(agents): classify web_fetch error pages as errors, not successful evidence
* fix(agents): classify 502/503/504 error shells as transient, not internal
Per review: a gateway error page is the try-a-different-source case, so
502/503/504 reason phrases now map to transient (warn, then escalate)
while 500/501 stay internal (stop). This mirrors _ERROR_RULES' own split
and removes the two phrases that classified differently through the
shell table than through _classify_error_text ("service temporarily
unavailable", "gateway timeout") — now pinned by a cross-path
consistency test over the live table plus a chain-level test that a 503
page warns and escalates instead of blocking web_fetch on first sight.
Also per review, _classify_error_shell returns a copy of the category
attrs instead of the rules-table dict, matching _classify_error_text.
Dropped "gone" from the phrase table: a single-word reason phrase
collides with legitimate one-word document titles ("# Gone"), and 410
error pages are rare while such titles are not; negative controls record
the decision.
* test(agents): pin crawl4ai's recorded renderer shape; note web_capture asymmetry
Per review: the title rule assumes every web_fetch provider renders
title-first. Eight of the nine providers hold by construction
(f"# {title}" or the shared Article.to_markdown()); crawl4ai renders
server-side, so its generator output (crawl4ai 0.9.2, fit and raw) was
measured over the same corpus and recorded as fixtures driven through
the real tool and middleware chain. A comment on _PAGE_CONTENT_TOOL_NAMES
records that web_capture's absence is intentional: its dead-target signal
is the provider warning suffix, which belongs to the provider boundary.
|
||
|
Aari
|
cd34a1a504
|
fix(skills): don't attach model tracing to the in-graph skill security scan (#4252)
* fix(skills): don't attach model tracing to the in-graph skill security scan * fix(skills): pass attach_tracing explicitly from the in-graph scan call site Follow the tracing INVARIANT's own convention rather than detecting the call context: scan_skill_content takes an attach_tracing flag, and _scan_or_raise -- the single in-graph choke point -- passes False. Standalone callers (Gateway skill routes, installer) keep the default True. The INVARIANT list named four sites and asks that new in-graph calls be added to it; record this fifth one so a future audit of that list finds it. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
lllyfff
|
de024646a7
|
feat(memory): memory template externalization (#4286)
* feat(memory): template externalization — externalized prompts + signal patterns Externalize two categories of hardcoded content in the DeerMem memory backend so operators can customise them without touching Python code. Prompt externalization (plugin 06): - 4 memory extraction prompts moved from Python string constants to YAML files under core/prompts/ (consolidation / fact_extraction / memory_update.chat / staleness_review). - load_prompt(name) and load_prompt_messages(name, variables) loaders. - memory_update uses the chat format (system / user message split) for prompt-caching-friendly system prefixes; other prompts stay text. - The extraction callback + per-agent prompt directories + Jinja2 dependency are intentionally not included (minimal surface). - Compatible with the upstream prompt additions from #4143 (expected_valid_days, staleFactsToExtend, KEEP/REMOVE/EXTEND in staleness review). Signal-pattern externalization (plugin 07, regex only): - Correction and reinforcement detection patterns externalised to core/message_patterns/{correction,reinforcement}.yaml. - load_patterns(name, patterns_dir) loader with bundled defaults. - detect_correction / detect_reinforcement accept a keyword-only patterns= parameter; signatures are backward-compatible. - patterns_dir config field added to DeerMemConfig. - Importance scorer (importance.py, build_importance_scorer, _prepare_update changes) is NOT included — deferred to a later PR. Co-Authored-By: Claude <noreply@anthropic.com> * fix(memory): fail loudly on invalid yaml in prompt/pattern loaders Replace silent error handling in load_prompt / load_prompt_messages / load_patterns so malformed yaml or missing required keys raise ValueError with the file path rather than a raw YAMLError traceback or silent empty-string / empty-list return. Changes: - load_prompt: YAMLError -> ValueError(path); missing/empty 'template' key -> ValueError - load_prompt_messages: YAMLError -> ValueError(path); missing/empty 'messages' key -> ValueError; KeyError from .format (placeholder mismatch) -> ValueError - load_patterns: YAMLError -> ValueError (was silent warn+return []). OSError still degrades (permission, not format). Not-a-list yaml -> ValueError (was silent warn+return []). Co-Authored-By: Claude <noreply@anthropic.com> * fix(memory): wire prompts_dir through updater, fail-loud patterns_dir, bump config_version Addresses PR feedback on the template-externalization PR: P1 — Wire prompts_dir into the DeerMem update path: - Add prompts_dir field to DeerMemConfig (default None = bundled defaults). - Thread it through DeerMem -> MemoryUpdater.__init__(prompts_dir=). - _build_staleness_section / _build_consolidation_section accept prompts_dir= keyword and call load_prompt() instead of relying on module-level shim constants. - _prepare_update_prompt passes prompts_dir to load_prompt_messages and to both section builders. - Add _PROMPT_CACHE to load_prompt so repeated lookups (once per memory-update cycle) do not re-read yaml from disk. P2 — Explicit patterns_dir must find its files: - When patterns_dir is explicitly set and a YAML file is missing, load_patterns() raises FileNotFoundError instead of silently caching an empty list. Bundled defaults (patterns_dir=None) still log a WARNING and return [] for a missing bundled file (packaging bug). P3 — Bump config_version and sync Helm: - config.example.yaml: 26 -> 27 - deploy/helm/deer-flow/values.yaml: 26 -> 27 - deploy/helm/deer-flow/README.md: 26 -> 27 - scripts/check_config_version.sh confirms parity. Tests: 224 passed (218 memory + 6 config_version). Lint: ruff check + ruff format --check pass. Co-Authored-By: Claude <noreply@anthropic.com> * fix(memory): cache load_prompt_messages, validate format fields, fail-loud load_patterns Review feedback from willem-bd: Cache for load_prompt_messages: Add _CHAT_TEMPLATE_CACHE + _render_messages() helper so the parsed chat templates are cached per (name, agent, prompts_dir). On cache hit only .format() rendering runs; the yaml file is read once per key. Validate format field in both loaders: load_prompt rejects format='chat' (redirects to load_prompt_messages); load_prompt_messages rejects format='text' (redirects to load_prompt). Prevents operators from loading a chat yaml via the text loader (or vice versa) without a clear error. Load_patterns fail-loud for explicit directories: - OSError (permission, etc.) now raises for explicit patterns_dir instead of silently disabling detection. - Malformed entries (missing/empty pattern key, wrong type) are skipped with a WARNING instead of silent skip. - Unknown flag names are warned instead of silently dropped. - re.error from compile() raises ValueError with file path and entry index instead of a bare re.error traceback. Co-Authored-By: Claude <noreply@anthropic.com> * fix(memory): thread agent_name through section builders, validate explicit prompts at construction, bump config_version to 28 P1 — Thread agent_name through staleness/consolidation section builders: _build_staleness_section and _build_consolidation_section now accept agent_name= and pass it to load_prompt(), so per-agent prompt overrides work for section templates too (not just memory_update). Previously only prompts_dir was threaded; agent_name was ignored for section builders. P1 — Validate explicit prompts at DeerMem construction: When DeerMemConfig.prompts_dir is explicitly set, DeerMem.__init__ now pre-loads all four prompt templates (staleness_review, consolidation, fact_extraction, memory_update with dummy variables) at construction time. A missing file, malformed YAML, or invalid placeholder raises immediately at startup instead of being caught by the updater's generic error handler and silently dropped as a failed update. P2 — Advance config_version to 28: Upstream main already consumed the previous 26→27 bump with a different schema change. Bump config.example.yaml, Helm values.yaml, and Helm README.md to 28 to version this PR's patterns_dir and prompts_dir additions. Co-Authored-By: Claude <noreply@anthropic.com> * fix(memory): render text templates at construction, propagate PromptConfigurationError Keep prompt-configuration failures out of the recoverable update catch: - Define PromptConfigurationError(ValueError) in prompt.py. Raised by load_prompt, load_prompt_messages, and _render_messages for bad yaml, missing keys, and invalid placeholders. - _do_update_memory_sync_impl, update_memory's executor path, and _process_queue all re-raise (PromptConfigurationError, FileNotFoundError, OSError) before the generic except Exception, so a bad explicit prompt is never silently returned as False. - DeerMem.__init__ prompt validation now renders text templates with dummy variables (.format(stale_facts="") etc.) so an unknown placeholder in staleness_review.yaml, consolidation.yaml, or fact_extraction.yaml is caught at construction. Co-Authored-By: Claude <noreply@anthropic.com> * fix(memory): drop re-raise guards, soften validation comment, exclude dormant fact_extraction - Remove (PromptConfigurationError, FileNotFoundError, OSError) re-raise guards from _do_update_memory_sync_impl, update_memory executor path, and _process_queue. The existing except Exception: logger.exception(...) already logs prompt-config errors at ERROR with full traceback; the re-raise was killing the entire batch and only surfacing via stderr. Per-agent override errors now log at ERROR per-item without aborting co-tenant updates. - Soften the construction validation comment to clarify it only covers global templates. Per-agent overrides are validated lazily at first use and logged at ERROR by the updater's exception handler. - Drop fact_extraction from construction validation (dormant prompt with no runtime caller; the shim + yaml remain for backward compat). Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Andrew Chen
|
4746a2579b
|
fix(loop-detection): clear the per-tool frequency counter on evict/reset (#4295)
`_evict_if_needed` and `reset` dropped `_tool_name_history` (the windowed
deque) but left `_tool_name_counter` (the Counter that mirrors it) in place.
After a thread id was LRU-evicted and later reused, its frequency count
resumed from the stale value instead of zero, so the first fresh tool call
was force-stopped ("Tool X called N times") as if the evicted calls had
never rotated out. `reset()` had the same gap.
Drop the counter alongside the deque at all three sites (evict, per-thread
reset, full reset). The window deque and its mirror Counter now stay in sync.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Huixin615
|
c9b6131f8f
|
fix(skills): reload mounted skills without restarting Gateway (#4264)
* fix(skills): add admin-only reload endpoint * fix(skills): preserve cache when reload fails |
||
|
Aari
|
5a5c661e9f
|
fix(middleware): recover malformed tool-call ids in dangling repair (#4246)
* fix(middleware): recover malformed tool-call ids in dangling repair DanglingToolCallMiddleware normalizes malformed tool-call names (#4008) and arguments (#4193) so strict OpenAI-compatible providers do not reject the next request. The id is the third field of that same recovery contract and was left alone. A provider that emits an empty id -- or omits it -- parses into a well-formed tool_calls entry, so it reaches the middleware through the normal path. The empty id never enters the pairing set, so the orphan pass drops the call's already-produced ToolMessage and the placeholder pass skips the call. The request then goes out carrying an empty id and with the real tool result gone. Normalize ids up front and re-point each already-paired ToolMessage at its call's new id, so the existing pairing/orphan/placeholder logic no longer sees a malformed id. Only the view that is actually read and serialized is relabelled; a valid id is left byte-for-byte alone, since it is matched verbatim against ToolMessage.tool_call_id. * fix(middleware): scope malformed-id result pairing to its own turn Malformed tool-call ids are all equally empty, so pairing recovered results by the original id alone was a global FIFO over the whole transcript. An earlier dangling call then consumed a later turn's result: the real result was served to the wrong call while the call that actually ran got the interrupted placeholder. An orphan result whose originating AIMessage was already gone could likewise be adopted by a later malformed call, resurrecting it instead of being dropped as the orphan pass intends. Walk the messages once in document order so only the most recent AIMessage's unanswered calls are claimable, which keeps a result answering the turn that issued it, and rule out the wrong parallel sibling within a turn by tool name. A result whose name matches no open call is left malformed for the existing orphan pass to drop rather than repurposed as some other call's answer. * fix(middleware): keep the shadowed raw view out of id recovery * fix(middleware): only claim a malformed result when the pairing is forced * docs(middleware): cite ToolNode's ordering guarantee for positional pairing |
||
|
Huixin615
|
f9340c1f08
|
test(mcp): cover passive skill tool visibility (#4247)
* test(mcp): cover passive skill tool visibility * test(mcp): tighten deferred discovery coverage |
||
|
HaotianChen616
|
756eac0d1a
|
feat(tool):Add structured synopsis for oversized tool output previews (#3377)
* Improve tool output preview synopsis * Add JSON path anchors to tool output synopsis * Fix JSON synopsis line anchors * fix(synopsis): tighten detectors and fix CSV first-row join Address review feedback from @willem-bd on PR #3377. Detectors: - _looks_yaml now requires >=3 key-shaped lines and refuses bare uppercase-tag lines ('INFO: ...', 'ERROR: ...') that look like log lines and would round-trip into a flat string dict via safe_load. Previously a 200-line log file was classified as 'YAML object with 3 top-level keys' and lost every line, count, and middle signal. - _try_yaml refuses payloads that safe_load collapses to a dict of all strings (the shape tracebacks and log lines collapse into). - _try_table applies the header-must-look-like-identifiers and minimum-row-count guards only to TSV, since the same safeguards would reject legitimate small CSVs. Refuses tab-indented bash output, ls -l listings, and tree dumps. Rendering: - CSV first data row is now rendered as a key=value list joined by ' | ' (e.g. 'name=Ada | description="a fine, brilliant logician" | score=98'). The previous delimiter.join(rows[1]) silently re-split cells that contained the delimiter inside a quoted cell, which made the synopsis report a 3-column table as 5 columns and misled the model about column count and content. Text summary: - _summarize_text now omits the closing excerpt entirely when the input is shorter than 2 * _TEXT_EXCERPT_CHARS, since the previous opener/closer slices overlapped and duplicated text for short inputs (build_tool_output_synopsis is reachable directly from tests and other callers that pass small inputs). Tests: - Update test_table_preview_extracts_columns to assert the new key=value list format. * fix(synopsis): drop JSON path line/byte offset hints The path-location hint was computed by string-searching for the quoted key in the original content and reporting its byte offset and line number. This anchors at the first textual occurrence of the key string, which is wrong when the key also appears as a value earlier in the document, or when the same key recurs at multiple depths. With nested paths the anchor drifts further on every step because the search cursor is advanced past each previous match. Concrete cases: content = '{"label": "items", "items": {"id": 1}}' _json_path_location(content, ['items']) -> ' (line 1, byte offset 10)' # the value, not the key content = '{"data": {"info": 1, "data": {"info": 2}}}' _json_path_location(content, ['data','data','info']) -> ' (line 1, byte offset 30)' # the inner first 'info', not the second The synopsis instructed the model to 'Start near the line hints above when present', so a wrong anchor would send read_file into the wrong region of the persisted .tool-results file. Drop the hint entirely. The path itself ('$.data.items') is already useful navigation; the agent uses read_file with start_line based on its own judgement of where the relevant slice is. Tests: - Update test_json_preview_reports_nested_paths to assert no 'line ' or 'byte offset ' appears in the body before the Access section. - Rename test_json_line_hints_use_original_content_offsets to test_json_paths_are_emitted_without_line_hints and invert the assertions to check the hints are absent. * fix(synopsis): bound _scalar_examples recursion depth Mirror the _JSON_STRUCTURE_DEPTH cap used by _json_container_paths and _json_shape so that deeply nested JSON cannot trigger RecursionError inside build_tool_output_synopsis. In ToolOutputBudgetMiddleware.awrap_tool_call the synopsis is built inside asyncio.to_thread(_patch_result, ...); a RecursionError would surface as a tool-call failure and the user would lose the entire output. 300-level nested JSON is well inside what an attacker-controlled MCP tool, a JSON-RPC-over-JSON-RPC chain, or a buggy serializer can produce. * feat(synopsis): restore inline raw head/tail sample The synopsis-only preview silently dropped the raw head/tail bytes that preview_head_chars / preview_tail_chars used to inline. For text/code/log outputs the agent lost first/last KB of the actual content and had to issue a follow-up read_file round-trip to see the trailing region (last paragraph of a fetched article, final error line in a traceback, closing diagnostics of a bash run). Restore an inline 'Raw sample (head + tail)' section in the preview. The section is composed by slicing head_chars from the start and tail_chars from the end of the content (with a '...' separator between them, and the tail suppressed when it would overlap the head). For binary-like output, the synopsis's own sample is reused unchanged. This makes preview_head_chars / preview_tail_chars operational again for every kind except binary, which already had a sample channel. Tests: - Rename test_json_preview_extracts_structure_instead_of_head_tail to test_json_preview_includes_structure_and_raw_sample and assert the raw sample section is present and the payload is reachable in the head slice. * test(synopsis): add regression tests for willem-bd review findings Add 8 regression tests under TestToolOutputSynopsis, one per finding in @willem-bd's review of PR #3377: - test_review_5_log_lines_are_not_misclassified_as_yaml Pins the YAML detector to refuse 'LEVEL: message' log lines. - test_review_6_json_paths_are_emitted_without_byte_offset Pins the removal of byte/line hint from JSON path descriptions. - test_review_7_scalar_examples_respects_depth_cap Pins that 500-deep nested JSON does not raise. - test_review_8_csv_first_row_quoted_cells_round_trip Pins the new key=value list format for CSV first-row rendering and asserts that quoted cells with embedded delimiters survive. - test_review_9_tsv_detector_rejects_tab_indented_bash Pins that tab-indented bash output is not classified as TSV. - test_review_10_preview_includes_raw_head_and_tail_sample Pins the restored inline 'Raw sample (head + tail)' section. - test_review_11_short_text_does_not_duplicate_excerpts Pins that closer is suppressed for inputs shorter than 2 * _TEXT_EXCERPT_CHARS. - test_review_12_preview_head_tail_chars_are_operational Pins that head_chars / tail_chars are wired into the rendered preview and not silently dropped. Also removes the now-stale 'byte offsets are approximate anchors' sentence from render_tool_output_preview's Access block; the synopsis no longer emits byte/line hints, so the guidance to 'start near the line hints' was misleading. * fix(synopsis): resolve lint errors on tool output budget tests Local 'make lint' on feat/tool-output-synopsis-preview (after fast-forward to current main) failed with three errors in tests added by PR #3377: - E501: 307-char bash_out literal in test_review_9_tsv_detector_rejects_tab_indented_bash - E741: ambiguous single-letter 'l' in test_review_11_short_text_does_not_duplicate_excerpts - E741: same ambiguous 'l' on the closing assert Replace the long literal with a join of per-row entries, rename the loop variable from 'l' to 'ln', and run ruff format on the two touched files to absorb the formatting drift introduced by the merge with main. Verification: - make lint -> All checks passed; 643 files already formatted - pytest tests/test_tool_output_budget_middleware.py -> 110 passed * fix: address willem-bd review findings (code/csv misclassification, text duplication, line snapping, dead constant, xml hardening, depth consistency) - _CODE_HINTS: require stronger signals for use/fn (trailing ; or parenthesised) - _try_table: apply _TABLE_MIN_DATA_ROWS gate to CSV too (not just TSV) - config.example.yaml: correct misleading comment about preview_head/tail_chars - _summarize_text: skip opener/closer excerpts when raw sample will be appended - _build_raw_sample: snap to line boundaries for clean truncation - Remove dead constant _TABLE_FIRST_ROW_CHARS - Prefer defusedxml for XML parsing (billion-laughs protection), fallback to stdlib - Replace _json_shape magic number 2 with named _JSON_SHAPE_MAX_DEPTH constant - Update tests to match new CSV gate (>=5 rows) and line-snapped sample counts * style: ruff format fix for tool_output_synopsis.py and test_tool_output_budget_middleware.py * fix(tool-output): address 4 review comments - DoS hardening + size cap 1. XML entity-expansion DoS: skip _try_xml when defusedxml is not available (SafeET is None), falling through to text + raw sample. (cid=3587721336) 2. YAML alias-bomb DoS: refuse to parse YAML content > 500 KB. (cid=3587721340) 3. Unbounded content parse: add _MAX_SYNOPSIS_INPUT_BYTES=5MB cap; oversized output falls back to raw head/tail sample instead of full parse. (cid=3587721346) 4. Scalar examples surface mid-document values: add docstring note that the synopsis is a structural summary, not a confidentiality filter. (cid=3587721353) * fix(tool-output): ruff format the synopsis string to one line --------- Co-authored-by: qinchenghan <qinchenghan@huawei.com> |
||
|
Huixin615
|
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 |
||
|
AochenShen99
|
94a34f382d
|
feat(context): record effective memory identity per run (#3556)
* feat(context): record effective memory identity per run * fix(context): address memory identity review feedback --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Tianye Song
|
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. |
||
|
ajayr
|
5c80c07dfe
|
fix(memory): treat explicit null backend_config values as omitted in DeerMemConfig (#4217)
config.example.yaml ships backend_config.model: as a bare key whose children
are all comments, which YAML parses to None (make config-upgrade then writes
an explicit model: null). DeerMemConfig.model is a non-Optional field with a
default, so from_backend_config(**{"model": None}) raised a ValidationError
and every run failed with "Input should be a valid dictionary or instance of
DeerMemModelConfig". Drop None entries in from_backend_config so YAML null /
empty keys fall back to field defaults, matching the documented "empty =
host default LLM" semantics. Upstream bug (#4122 schema); regression-pinned
in test_deermem_self_contained.py.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
||
|
Vanzeren
|
1769b2de0d
|
fix: read run stop_reason from runtime context (#4188)
* fix: read run stop_reason from runtime context * fix: address review feedback for #4188 stop_reason integration - migration 0005: use safe_add_column for consistency and drift detection - worker: clear runtime.context stop_reason at start of each _stream_once turn so a clean continuation doesn't inherit a prior cap reason - tests: replace circular unit test with real middleware integration tests that exercise LoopDetectionMiddleware._apply and TokenBudgetMiddleware._apply through the worker, proving the full middleware → runtime.context → persist pipeline * fix(test): resume conftest * fix: stamp stop_reason in all guard middlewares, fix clearing semantics |
||
|
Huixin615
|
3247f61750
|
fix(middleware): sanitize invalid tool call arguments (#4193)
* fix(middleware): sanitize invalid tool call arguments * refactor(middleware): share tool argument parsing |
||
|
Tu Naichao
|
959bf13406
|
fix(memory): flush memory queue on graceful shutdown to prevent loss (#4181)
* fix(memory): bounded shutdown flush via MemoryManager.shutdown_flush Re-applies the memory-queue shutdown drain on top of the pluggable MemoryManager abstraction (#4122): the old top-level MemoryUpdateQueue singleton is gone, so the drain is now a backend contract instead of host code reaching into the queue. - MemoryManager ABC: shutdown_flush(timeout) -> bool. Every backend implements a bounded graceful-shutdown drain. - DeerMem: queue.flush_sync (daemon-thread + Event.wait hard timeout for the uninterruptible sync LLM call; joins an in-flight worker first so contexts a debounce Timer already pulled out are not lost on exit; skips inter-item sleep on the drain path; per-item succeeded/failed count), exposed via shutdown_flush. - noop: shutdown_flush is a clean no-op success. - Gateway lifespan: call get_memory_manager().shutdown_flush(timeout) after channels/scheduler stop, via asyncio.to_thread, try/except bounded. No host-level pending/processing guard -- the backend short-circuits on an idle buffer, so the host cannot "forget" the in-flight case (structurally eliminates the guard race flagged on the prior revision). - shutdown_flush_timeout_seconds added to the shared MemoryConfig (host-owned lifecycle budget, default 30, 1-300) + exposed on MemoryConfigResponse and the embedded client; config_version 25 -> 26. Tests: queue flush_sync (7), lifespan drain incl. False-branch caplog assertion + disabled gate (3), ABC contract noop/deermem (3). * fix(chart): gateway grace period so memory drain is not SIGKILLed K8s defaults terminationGracePeriodSeconds to 30s, shorter than the Gateway's graceful-shutdown work (channel stop ~5s + memory queue drain default 30s). Without an explicit grace period, K8s SIGKILLs the memory drain mid-flight and silently re-introduces the loss shutdown_flush is fixing (flagged on the prior revision). - gateway pod: terminationGracePeriodSeconds (default 45, configurable). - gateway container: preStop sleep (default 5, 0 disables) so the Service/ingress deregisters the pod before SIGTERM begins the drain. - values.yaml + README: both configurable; README documents that the grace period must track memory.shutdown_flush_timeout_seconds. * docs(memory): document shutdown_flush_timeout_seconds + lifespan drain Add the host-shared field to the memory config list and Config Schema summary in backend/AGENTS.md, noting the lifespan drain and the K8s grace-period relationship. * fix(chart): bump embedded config_version to 26 The chart's embedded `config:` block (values.yaml + README example) still had config_version: 25 after commit f3ca8e9f raised config.example.yaml to 26, failing the validate-chart config_version drift check. Bump both to 26. |
||
|
Xinmin Zeng
|
16919f7c52
|
fix(skills): reuse the resolved app config in the no-arg skills prompt section (#4160)
get_skills_prompt_section() without app_config resolved get_app_config() only to read container_path, then let the enabled-skills load fall back to the warm cache. On a cold start the cache is empty and the first call returns an empty skills list while the synchronously-loaded disabled section is populated, so manually assembled agents (create_deerflow_agent style integrations) got a prompt with no enabled skills. Rebind the resolved config so the storage and enabled-skills loads below use it too; when no config is resolvable the cache-only fallback is unchanged. Adds a cold-cache regression test. Fixes #4144 Co-authored-by: fancyboi999 <fancyboi999@users.noreply.github.com> |
||
|
qin-chenghan
|
ad45f59d66
|
feat(memory): pluggable memory abstraction with self-contained DeerMem backend (#4122)
* feat(memory): pluggable + self-contained memory system (MemoryManager plan phases 1 & 2) Phase 1 — Pluggable (steps 0-10): - ABC MemoryManager (9 methods) + singleton factory + drop-in backend discovery - DeerMem default backend with core/ (storage/queue/updater/prompt/message_processing) - NoopMemoryManager backend (proves pluggability) - All call sites (middleware/hook/prompt/gateway/client/app) routed through manager - hasattr capability probing for DeerMem-internal methods (no hard imports) - MemoryConfig gains manager_class field; shared vs DeerMem-private annotated Phase 2 — Self-contained DeerMem (steps 11-18): - backend_config passthrough + DeerMemConfig (all DeerMem-private fields moved off MemoryConfig) - DI: DeerMem owns storage/queue/updater/llm as instance attributes (no global singletons) - Storage independence: core/paths.py with own root (~/.deermem or ), factory auto-injects deer-flow's runtime_home() as absolute base_dir (zero-config) - LLM independence: core/llm.py via langchain init_chat_model (no create_chat_model) - Trace independence: optional tracing_callback replaces inject_langfuse_metadata/request_trace_context - Message processing independence: hide_from_ui default-skip + optional should_keep_hidden_message hook - Internal imports → relative (only deer_mem.py ABC import is host-relative) - Carrier (deer_mem.py adapter) / portable (deermem/ config+core) split - New tests: test_deermem_self_contained + test_memory_manager_pluggable; all memory tests migrated - Other-agent demo: samples/other_agent_demo/ + automated portability test - config.example.yaml memory section updated to phase-2 schema * feat(memory): port consolidation + staleness fix into self-contained DeerMem; phase-2 host hooks Port upstream #3996 (memory consolidation) and #3993 (staleness KeyError fix) from origin/MemoryManager into the pluggable, self-contained DeerMem structure (backends/deermem/deermem/), adapted to the DI MemoryUpdater (config injected, not get_memory_config globals): - DeerMemConfig: add consolidation_enabled (opt-in, default false) / consolidation_min_facts / consolidation_max_groups_per_cycle / consolidation_max_sources - prompt.py: factsToConsolidate JSON field + {consolidation_section} placeholder + CONSOLIDATION_PROMPT constant - updater.py: _coerce_source_confidence / _select_consolidation_candidates / _build_consolidation_section module helpers (matching the existing _select_stale_candidates style); consolidation normalization in _normalize_memory_update_data; consolidation apply in _apply_updates (after max_facts trim, with apply-time guardrails mirroring staleness); staleness KeyError fix (f["id"] -> f.get("id") is not None) applied to both the staleness guardrail and the consolidation allowed_source_ids comprehension - config.example.yaml: consolidation section under memory.backend_config - tests/test_memory_consolidation.py: 40 DI-adapted tests (running, not skipped) incl. the staleness KeyError regression Also includes in-flight phase-2 host-integration work: storage_path semantics (any absolute/relative value = root dir) and host-default tracing_callback / should_keep_hidden_message hooks injected into backend_config by the factory. Co-Authored-By: Claude <noreply@anthropic.com> * feat(memory): add noop backend template and backends guide - backends/noop/: complete drop-in template (config.py with zero deer-flow imports, noop_manager.py with a 6-step new-backend walkthrough in its docstring, commented optional fact-CRUD capabilities). - backends/README.md: which files to touch when adding/swapping a backend, the 5-item backend contract, and common pitfalls. - manager.py: generalize backend examples in comments (drop mem0-specific references). Co-Authored-By: Claude <noreply@anthropic.com> * fix(frontend): guard formatTimeAgo against invalid timestamps Return a neutral placeholder when the input date is invalid (e.g. an empty lastUpdated from a backend with no memories) instead of throwing 'Invalid time value' from date-fns. Co-Authored-By: Claude <noreply@anthropic.com> * feat(memory): wire tool-driven memory mode through the MemoryManager ABC tools.py (memory_search/add/update/delete) now calls get_memory_manager() instead of the removed host memory module, so tool mode (memory.mode: tool) works for any backend. DeerMem.search is implemented (case-insensitive substring match, ranked by confidence) as a stand-in for the planned semantic retrieval; noop.search returns [] (unchanged). Fact-CRUD tools use getattr+callable probing -- backends lacking those ops (noop) get a clear JSON error instead of crashing. Tests: test_memory_tools rewired to mock the manager (handler tests) + TestModeGating retained; test_memory_search now covers DeerMem.search; pluggable stubs test updated (search no longer a stub). Co-Authored-By: Claude <noreply@anthropic.com> * fix: resolve lint errors (import sorting, type annotation quotes, E402 in skipped tests) * docs: restore explanatory comments in config.example.yaml memory section * fix(security): port html-escape memory facts fix (#4097) to vendored DeerMem prompt.py * fix(memory): address review + port dropped upstream memory fixes Review blockers (vendored DeerMem): - #4044 restore _escape_memory_for_prompt (current_memory blob in MEMORY_UPDATE_PROMPT) - prevents </current_memory> breakout - #4028 html.escape staleness-section cat/content in _build_staleness_section - #4119 add _escape_summary for injection-path summaries (Work/Personal/ Current Focus/Recent/Earlier/Background) - default-model silent no-op: factory injects host default chat model via a new host_llm slot (create_chat_model(name=None)); DeerMem prefers host_llm over build_llm(model). Zero-config extraction works out of the box again - MemoryConfigResponse: fix stale docstring (backend-agnostic shape; DeerMem knobs live under backend_config, not top-level - restoring flat would re-couple the API to DeerMem). Frontend audited: does not read /memory/config - _host_default_tracing_callback: restore langfuse assistant_id/environment - search: push category onto the ABC signature; DeerMem filters BEFORE the top_k slice (was filtered client-side after slicing -> starved results) - _do_update_memory_sync: split into wrapper+impl; bind trace_id into the request-trace ContextVar on the Timer/executor worker via a new trace_context_manager host hook (None trace_id left unbound - no fabrication) - client.py fact-CRUD now passes user_id (was writing to the global bucket while get_memory reads per-user) - _resolve_manager_class: fail-fast (raise ValueError) on an unresolved explicit manager_class instead of silently falling back to DeerMem (memory is persistent state - a wrong store is a silent data-integrity footgun) Upstream memory fixes dropped by the host->vendored rename conflict, re-ported to backends/deermem/deermem/core/ (+ deer_mem.py): - #4073 queue busy-timer-spin -> _reprocess_pending flag (core/queue.py) - #4074 null source.confidence in staleness -> _coerce_source_confidence (core/updater.py: _build_staleness_section + _apply_updates stale sort) - #4075 factsToRemove is optional (drop from _REQUIRED_MEMORY_UPDATE_TOP_LEVEL_KEYS) - #4076 null confidence in search ranking -> _coerce_source_confidence (deer_mem.py DeerMem.search) host_llm + trace_context_manager are host-injected via backend_config (factory in manager.py), keeping backends/deermem/ at exactly one `from deerflow` line (the ABC contract) - portability test preserved. Co-Authored-By: Claude <noreply@anthropic.com> * fix: resolve lint errors (F541 f-string without placeholders, E501 line too long) * fix(memory): restore hide_from_ui clarification preservation, expose mode Two memory-system fixes (F541/E501 lint was already fixed on this branch): - filter_messages_for_memory: restore default preservation of well-formed human_input_response clarification answers (v2 regression). The self-containment refactor made the bare function skip ALL hide_from_ui when no hook was passed, but upstream preserves well-formed clarification responses by default (test_hide_from_ui_human_input_response_is_preserved). Inline a host-agnostic _is_human_clarification_response mirror of read_human_input_response as the default keep-decision; the host-injected should_keep_hidden_message hook still overrides (production path unchanged). Portable package stays zero `from deerflow`. - /memory/config: expose `mode` (middleware|tool) in MemoryConfigResponse + the config/status endpoints + client.get_memory_config. mode is a host- shared, behavior-determining field missing from the response projection. Sync tests (mock .mode; e2e assert mode present). - Align manager_class field docstring with fail-fast behavior. Tests: filter/self-contained/portability (35) + memory-config (4) pass; ruff clean. Co-Authored-By: Claude <noreply@anthropic.com> * fix(memory): resolve ruff format failures in memory module + tests `make lint` runs `ruff format --check` in addition to `ruff check`; 8 memory files had pending format changes -- 7 pre-existing (deer_mem, updater, tools, test_memory_queue/router/search/tools) + message_processing from the hide_from_ui fix. Apply `ruff format`: whitespace/wrapping only, no logic change. 109 memory tests pass; ruff check + format --check both clean. Co-Authored-By: Claude <noreply@anthropic.com> * fix(memory): address PR review - legacy field migration, fact_id contract, path/docs Address willem-bd's review on PR head bc8bf0d4 (risk:high, persistent state): - config: auto-migrate pre-abstraction top-level memory.* DeerMem fields (storage_path, max_facts, debounce_seconds, model_name, token_counting, staleness_*, consolidation_*) into backend_config on load + warn, so an upgrade does NOT silently revert customized settings (was: silent extra='ignore' drop). model_name -> backend_config.model.model. Unknown top-level keys warned. - factory: resolve a relative backend_config.storage_path against runtime_home() (base_dir-relative, CWD-independent) to preserve pre-abstraction semantics; paths.py stays portable (no runtime_home import). - tools: memory_add uses the fact_id returned directly by create_fact instead of re-deriving it via content-key matching (coupled the tool to the backend's content normalization; could misreport a storage cap). create_fact now returns (memory_data, fact_id); gateway/client/tool updated. Fix terse {"error":"content"} -> {"error":"empty content"}. - app.py: update stale token_counting=="char" warm-up comment to point at manager.warm (DeerMem.warm re-checks char and returns early). - router: comment explaining reload_memory silent fallback vs fact 501 asymmetry (read-only degrade vs write fail-loud). - CHANGELOG: document breaking changes (/memory/config + client.get_memory_config shape flat->backend_config; custom storage_class path moved + __init__ must accept config) and the legacy-field auto-migration. - tests: add regression test pinning the per-user memory path ({storage_path}/users/{safe_user_id}/memory.json == host make_safe_user_id) across the abstraction; update create_fact mocks for (memory_data, fact_id). Tests: 273 passed (memory suite); ruff check + format clean. Co-Authored-By: Claude <noreply@anthropic.com> * fix(memory): address PR review - storage_path, max_facts, tracing, parsing Six review findings (willem-bd), each verified against upstream: - storage_path semantics (file -> root dir): migration drops file-style (.json) legacy values with a warning; factory raises if storage_path resolves to an existing file (avoid silent NotADirectoryError write failure). CHANGELOG + config.example.yaml comment updated. - create_memory_fact enforces max_facts again (via _trim_facts_to_max) and returns (memory, None) when the cap evicts the new fact; memory_add tool reports "not stored", client raises ValueError, POST /memory/facts -> 409. - max_facts trim uses _coerce_source_confidence (was raw f.get("confidence", 0) -> TypeError on non-float imported/legacy confidence, swallowed as silent update failure). - memory-tracing assistant_id restored to "memory_agent" (was "lead-agent" copy-paste; matches upstream + DeerMem run_name). - _is_human_clarification_response cross-checked against read_human_input_response (drift guard test). - empty-string legacy values skipped silently in migration (narrow fix, not broad "if not value" which would skip explicit bool False). 8 new regression tests. make lint + 406 memory tests pass. Co-Authored-By: Claude <noreply@anthropic.com> * fix(memory): address internal review - storage fail-fast, build_llm degrade, config warn, noop template Addresses 4 findings from the PR #4122 internal supplemental review (parallel to willem-bd's review, no overlap): - create_storage fail-fast: a misspelled/unimportable storage_class now raises ValueError instead of silently falling back to FileMemoryStorage. Memory is persistent state, so a wrong store is a data-integrity footgun; mirrors the existing manager_class resolution policy. (storage.py) - noop template create_fact signature: the commented template used keyword-only `content` and returned a bare dict, while DeerMem's actual create_fact takes positional `content` and returns tuple[dict, str|None] (the memory_add tool passes content positionally; gateway/client/tools all tuple-unpack). A backend copied from the template would 500 on fact-CRUD. Template fixed; delete_fact/update_fact templates left (callers compatible). (noop_manager.py) - build_llm graceful degrade: wrap init_chat_model in try/except, degrade to None + WARNING on failure (mirroring _host_default_llm) so a misconfigured explicit model does not crash app startup -- non-LLM memory ops still work and an update raises at runtime with the error logged. (llm.py) - from_backend_config unknown-key warning: log a WARNING for unknown backend_config keys (mirrors the host layer's load_memory_config_from_dict) so a typo like `storage_pat` does not silently fall back to the default and write memory to an unintended location. (config.py) Tests: rewrote 3 create_storage fallback tests to expect ValueError; added 4 tests (build_llm zero-config/degrade, from_backend_config warn/silent). make lint green; full memory suite passes. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: lllyfff <2281215061@qq.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: lllyfff <122260771+lllyfff@users.noreply.github.com> |
||
|
Aari
|
8e96a6a252
|
fix(security): html-escape the conversation block in MEMORY_UPDATE_PROMPT (#4162)
format_conversation_for_update embeds raw user turns into the <conversation> slot of MEMORY_UPDATE_PROMPT. This is the most attacker-influenced input in the prompt, and it was unescaped: a message containing "</conversation><current_memory>..." closes the conversation block and forges a <current_memory> authority section for the extraction LLM, which can be steered into persisting an arbitrary high-confidence fact — and that fact is later injected into the lead-agent system prompt's <memory> block, which the prompt declares trusted. This is the last unguarded sibling of a rule the repo has established repeatedly. #4044/#4060 html-escaped the current_memory slot of this exact template; #4097 escaped the <memory> injection renderer. In updater.py the same .format() call escapes current_memory and leaves conversation raw. The memory updater sees raw text because InputSanitizationMiddleware only rewrites the ModelRequest and never mutates state, while MemoryMiddleware queues the raw state messages. Escape content with html.escape(quote=False), mirroring _escape_summary / _format_fact_line — after truncation so a trailing "..." cannot split an entity, on both human and assistant turns. Render-time only: no stored value is mutated, so the apply path is unaffected. The conversation function already strips <uploaded_files> here, so tag hygiene in this renderer is established. Scope is the memory updater. The summarizer's <new_messages> / <existing_summary> blocks are the same rule unguarded, but their output is quarantined as untrusted durable context rather than promoted to system authority; that hardening will be a separate change. |
||
|
qin-chenghan
|
713ee544b7
|
fix(agents): stop persisting base64 image data in checkpoint state (#4140)
* fix(agents): stop persisting base64 image data in checkpoint state (#4138) The viewed_images state field stored full base64-encoded image data, which was duplicated across every subsequent checkpoint (O(n * steps) growth). A single 1MB image viewed early in a conversation would be re-stored in every checkpoint for the rest of the session. Changes: - ViewedImageData: replace base64 field with lightweight metadata (mime_type, size, actual_path) - view_image_tool: store only metadata in state, no base64 encoding - ViewImageMiddleware: read image files from disk on-demand in before_model and encode base64 temporarily for the model call - Update all tests to use the new metadata-only format This is the first step of #4138. The base64 data is no longer in persistent state, but the injected HumanMessage (with base64 content) still appears in the checkpoint for the step where it was injected. Checkpoint retention policies and large tool result dedup are separate follow-up items. * fix(agents): address review feedback on #4140 - view_image_tool: remove stale 'convert to base64' comment, replace with 'validate contents'; drop redundant image_size reassignment and add a TOCTOU guard that rejects files changed between stat() and read(). - view_image_middleware: extract _read_image_as_data_url helper that re-checks size against the recorded value AND the absolute cap (_MAX_IMAGE_BYTES). Document the trust assumption for actual_path (server-set, not client-settable) in the helper docstring. - view_image_middleware: abefore_model now runs the blocking read+encode via asyncio.to_thread to avoid stalling the event loop on up to 20MB images. - tests: add coverage for OSError during read, file-changed-since-view (TOCTOU), and size-exceeds-cap branches. |
||
|
qin-chenghan
|
60e50537f3
|
fix(subagents): prohibit task tool in general-purpose system prompt (#4161)
* fix(subagents): prohibit task tool in general-purpose system prompt (#4159) The general-purpose subagent correctly lists `task` in disallowed_tools to prevent recursive nesting. However, the system prompt did not explicitly tell the LLM that `task` is unavailable. When the subagent sees the parent agent use `task`, it infers the tool is available and attempts to call it, triggering a LangGraph tool validation error. Add an explicit <tool_restrictions> block to the system prompt stating that `task` is NOT available and the subagent must NEVER attempt to call it. This prevents the LLM from attempting the call in the first place, rather than relying on runtime rejection. Add a regression test verifying the prompt contains the prohibition. * fix(security): register tool_restrictions in input sanitization denylist PR #4161 added <tool_restrictions> to general_purpose.py subagent prompt but did not register it in _BLOCKED_TAG_NAMES. The anti-drift test test_denylist_covers_framework_authority_blocks caught this: forging <tool_restrictions> in untrusted input could trick the model into believing it has (or lacks) tool restrictions it does not. Add 'tool_restrictions' to _BLOCKED_TAG_NAMES alongside the other subagent authority blocks (file_editing_workflow / guidelines / output_format / working_directory). |
||
|
黄云龙
|
9c77046d10
|
fix(middleware): drop orphan ToolMessages so strict providers don't 400 (#4080)
* fix(middleware): drop orphan ToolMessages with no matching AIMessage tool_call The rebuild loop only skipped ToolMessages whose tool_call_id matched a known AIMessage tool_call (to be re-emitted after it). An orphan ToolMessage whose tool_call_id has no matching AIMessage tool_calls fell through and was kept, leaving a dangling tool result that strict providers reject. Drop orphan ToolMessages as well, logging at debug. * fix(dangling): demote orphan-drop logs, add tool_call_id=None test - Update module/class docstrings to mention orphan ToolMessage handling - Accumulate orphan drop_count and emit a single logger.warning instead of per-message logger.debug calls - Simplify early-return logic: return None only when no patching AND no orphans were dropped - Add test_tool_call_id_none_orphan_is_dropped — a ToolMessage with tool_call_id=None is always an orphan and must be dropped Closes #4080 Co-Authored-By: Claude <noreply@anthropic.com> * fix(test): use model_construct for None tool_call_id test to bypass pydantic validation ToolMessage content='ghost' tool_call_id=None fails pydantic validation at construction. Use model_construct to simulate a corrupt/edge-case payload without tripping the string-only guard. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Aari
|
c57cf221d3
|
fix(security): html-escape the summary input blocks in the summarization prompt (#4182) | ||
|
黄云龙
|
e492bb1c68
|
fix(middleware): window LoopDetection tool-frequency counter so long runs don't false-trip (#4072)
* fix(loop-detection): decay per-tool frequency counter with a windowed deque The Layer 2 per-tool-type frequency guard in _track_and_check used a monotonic integer counter (freq[name] += 1) that never decayed or reset, so a long-running thread could trip the frequency warn/hard-stop even when calls were spread out over the whole run. Replace it with a deque of recent tool names trimmed to window_size, matching the windowed hash layer, and count occurrences within the window. Update _evict_if_needed and reset() to manage the new _tool_name_history storage. * address review: size Layer-2 freq window to the hard limit, not window_size The windowed freq_count is bounded by the deque length; reusing Layer-1's window_size (default 20) capped it below tool_freq_warn (30) / hard (50), making the Layer-2 guard dead code under the shipped default config. Size a dedicated _tool_freq_window = max(window_size, tool_freq_hard_limit, override hard limits) so a tight burst reaches the limit while spread-out calls still decay. Per @willem-bd review on #4072. Adds default-config regression tests: freq window >= hard limit, override coverage, and a tight-burst-with-distinct-args hard-stop under real defaults. Co-Authored-By: Claude <noreply@anthropic.com> * fix(#4072): docstrings describe windowed semantics; defaultdict+Counter for O(1) Addresses willem-bds three inline nits: 1. Docstrings for tool_freq_warn/tool_freq_hard_limit now explain the sliding-window semantics and reference _tool_freq_window sizing. 2. Hot-path deque() allocation avoided: _tool_name_history uses defaultdict(deque) instead of dict.setdefault(thread_id, deque()). 3. O(window) sum() scan replaced with mirrored collections.Counter (incremented on append, decremented on popleft) for O(1) freq_count. --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Aari
|
41b137c4c4
|
fix(security): block forged framework tags in the input guardrail (#4155)
* fix(security): block forged framework tags in the input guardrail
InputSanitizationMiddleware's _BLOCKED_TAG_NAMES neutralizes forged
framework tags in untrusted input, but missed soul, thinking_style, and
critical_reminders -- which the lead-agent system prompt's System-Context
Confidentiality section names as internal framework data -- and the
underscore spelling system_reminder emitted by the todo/terminal
middlewares (only the hyphen spelling was blocked). A user, or an
attacker-controlled web_fetch/web_search page via the shared
neutralize_untrusted_tags primitive, could forge these blocks. Add them.
* fix(security): cover framework authority blocks as a class, not a subset
The confidentiality section declares every framework structured tag trusted
("and all other structured tags"), so the denylist must cover the authority
blocks as a class. Add the live blocks still passing both sanitization paths
(clarification_system, self_update, response_style, citations, skill_index,
available_skills, disabled_skills, memory_tool_system, durable_context_data,
slash_skill_activation), and pin the set against drift with a test that scans
the framework source and fails when a new block is not blocked.
* fix(security): scan the whole harness for framework blocks, fail closed
The drift guard added in the previous revision scanned a hand-listed set of
source files. That is the same forgot-to-update-a-list root cause the guard was
meant to eliminate, one level up, and it failed exactly that way: tool_search.py
was not in the list, so <mcp_routing_hints> and <available-deferred-tools> —
both rendered into the lead-agent system prompt via the {deferred_tools_section}
/ {mcp_routing_hints_section} placeholders — passed both sanitization paths
unneutralized.
Replace the file list with a repo-wide scan plus an exemption set that states a
reason per tag. The point is the failure direction, not the breadth: a new
framework block anywhere in the harness now turns CI red until it is either
blocked or exempted on the record, where before a block emitted from an unlisted
file was silently unguarded.
The scan reads raw source rather than AST string literals on purpose: an
attributed block built as an f-string splits its '>' into a separate literal
chunk, so an AST-on-literals scan misses it (verified against
<consolidation_candidates>). Raw source has one comment false positive, exempted.
Exempted with reasons: leaf/wrapper elements; the memory-updater and summarizer
prompts, which are built from checkpointed state rather than the ModelRequest
this middleware rewrites, so blocking them here would be false coverage, not
protection; and the MindIE provider wire format, parsed out of model output.
The scan surfaced five further live authority blocks beyond the two reported.
Subagents reuse _build_runtime_middlewares and therefore share this denylist, so
their system-prompt blocks are in the same class: file_editing_workflow,
guidelines, output_format, working_directory. goal_continuation is a
framework-authored hidden HumanMessage injected into the lead agent.
Also loosen the scanner regex to match the tolerance of _BLOCKED_TAG_PATTERN so
an attributed block cannot hide from the guard.
|
||
|
AnoobFeng
|
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 |
||
|
Yufeng He
|
e361122b9a
|
fix(security): html-escape subagent descriptions before the <subagent_system> block (#4157)
A custom subagent's description is agent-editable (persisted by setup_agent / update_agent) and is rendered into the <subagent_system> block of the lead-agent system prompt via the available-subagents listing. It was interpolated raw, so a first line like "</subagent_system><system-reminder>..." could close the block and forge a framework-reserved tag inside the system-role prompt. Escape it with html.escape at the render site, matching the sibling fixes for <soul> (#4137), memory facts (#4097), skill metadata (#4128), and remote content (#4099/#4002). Built-in descriptions are trusted constants and stay untouched. Adds a red/green regression test mirroring test_soul_prompt_injection.py. |
||
|
Yufeng He
|
807c3c5218
|
fix(security): html-escape SOUL.md before it enters the <soul> prompt block (#4137)
SOUL.md is agent-editable (setup_agent / update_agent persist it) and get_agent_soul renders it into the <soul> block of the lead-agent system prompt without escaping. A crafted personality such as "</soul></system-reminder>\n\nSYSTEM: ..." can close the block and relocate the text after it out of the trust zone the system prompt declares — the same break-out the skill/memory/tool-result escaping in #4097/#4119/#4128/#4099 already closes at their render sites. <soul> is the remaining one, and it lands in the highest-trust system-role block. Escape with html.escape(quote=False) (element-text position, never an attribute). Adds a regression test that fails on main. Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com> |
||
|
DanielWalnut
|
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> |
||
|
Daoyuan Li
|
2fa0505070
|
fix(skills): activate a slash skill once per run, not per model call (#4103)
* fix(skills): activate a slash skill once per run, not per model call SkillActivationMiddleware injects the activation reminder for a slash command via request.override(messages=...), which LangChain's create_agent uses for a single model call and never writes back to graph state. The dedup guard scans request.messages for a prior reminder, but model_node rebuilds request.messages fresh from persisted state on every tool-loop step, so the reminder is never present on the 2nd..Nth model call of a turn. Every model call therefore re-parsed the command, re-read SKILL.md from disk, re-injected the multi-KB body, and re-recorded an "activate" audit event, despite the code intending a single activation per run (#3861 semantics: one activation call, many follow-up model calls). Key the dedup off the run context instead, which LangGraph threads through every model-node call of a run (the same durable signal the request-scoped secret source already uses). The activation call records the slash message's identity in context; later calls for the same message skip re-activation. A new user slash message keys differently and still activates. Secret binding is unaffected: it already re-resolves from the persisted slash source on every call. Adds regression tests that rebuild the real multi-call turn state and assert a single activation across the tool loop, plus a test proving a new slash command still activates. * fix(skills): address review nits on run-scoped activation dedup - Extract _already_activated(run_context, run_key) so the dedup check mirrors the existing _has_existing_activation_for_target sibling instead of an inline dense conditional. - Compute _activation_run_key() once in _find_activation_target and thread it through _prepare_model_request instead of recomputing it at the write site, making the "same key for check and write" invariant explicit in the code rather than implicit. - Document why the run-context write is an overwrite rather than an append/set: only the latest real user message is ever considered an activation target, so there is nothing earlier in the run worth preserving. - Add a regression test locking in the degraded-path contract: when runtime.context is None, the middleware still activates per-call instead of crashing or wrongly no-op'ing. |
||
|
Aari
|
42544755ac
|
fix(skills): escape untrusted skill metadata before it enters the model prompt (#4128)
* fix(skills): escape untrusted skill metadata before it enters the model prompt Skill name/description/allowed-tools come from the frontmatter of a user-installable .skill archive (POST /api/skills/install or a drop into skills/custom/); the parser only strips them. The slash-activation and durable-context siblings already html.escape these exact fields before rendering them into a model-visible block -- but five other render sites emit them raw. The sharpest is the default path, <available_skills> in the system prompt (skills.deferred_discovery: false): a community skill whose description closes the block can forge a framework-trusted <system-reminder> into the lead-agent system prompt. Driven through the real apply_prompt_template(), the forged tag reaches the system prompt raw on main and is neutralized here. Escape at every render site that emits untrusted skill metadata/content: - <available_skills> (name/description/location) and <disabled_skills> (name) in lead_agent/prompt.py; - describe_skill output (name/description/allowed-tools/location) and <skill_index> (name) in skills/describe.py; - the subagent <skill name=...> attribute plus the raw SKILL.md body in subagents/executor.py::_load_skill_messages -- its direct sibling skill_activation escapes both, this escaped neither. quote=False in element-text positions (matching skill_context and the #4097 correction), quote=True in the one attribute position (matching skill_activation). category is a controlled enum and is left as-is; escaping is render-time only, so stored skills are unchanged and re-rendering never double-escapes. * fix(skills): escape skill name in the slash-activation prose line The slash-activation reminder emitted `activation.skill_name` raw in its prose line while escaping the same value in the adjacent <skill name="..."> attribute. skill_name is grammar-gated to [a-z0-9-] by resolve_slash_skill before it reaches the renderer, so this is a defense-in-depth / consistency fix rather than a reachable injection: the two positions can never drift if a future caller builds an activation from an unconstrained name. Reuse the already-computed escaped_skill_name. |