mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-09-18 18:46:17 +00:00
157 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
阿泽
|
ea74367502
|
fix(runtime): honor LangGraph Server identity for user-scoped data (#4538)
* fix(runtime): honor LangGraph Server identity for user-scoped data * fix(runtime): scope custom agent SOUL by resolved user |
||
|
Aari
|
9a43d8276d
|
fix(gateway): replay edit and rerun from a settled checkpoint (#4534)
Editing the only turn of a thread reran the original prompt: the model
answered the question the edit was replacing while the UI showed the
edited text, and the edit vanished on reload.
The replay-base lookup decided whether a checkpoint predates the target
user message by message id alone. DynamicContextMiddleware re-keys the
first user turn to `{id}__user` mid-run, so every checkpoint written
before it holds the same prompt under an id the lookup cannot match. The
scan walked past those and anchored inside the run that produced the
turn — a checkpoint that still contains the original prompt and owns the
injection node's pending writes, which the replay then re-added after the
edited message.
Require the replay base to be a settled checkpoint (no pending tasks) in
both the lineage walk and the chronological fallback. That rule is
middleware agnostic: the first turn now anchors on the thread's empty
initial checkpoint and later turns on the previous run's tail, which also
drops the existing reliance on LangGraph discarding a stale `__start__`
write.
Edit replay additionally passes `head_checkpoint` so it resolves its base
lineage-first like regenerate does, and a replayed user message is
restored to its pre-swap id: replaying `{id}__user` into a state that has
no reminder yet makes the middleware treat the turn as already injected
and silently drops its date and memory block.
Frontend: a prepared replay masks the turn it supersedes, so the
optimistic-message baseline is taken from the post-mask human count. The
pre-mask count can never be exceeded when the replay puts exactly one
human message back, and on the first turn the runtime re-keys the
replacement message so identity comparison cannot stand in for the count.
Fixes #4531
|
||
|
qin-chenghan
|
88f9f81877
|
fix(clarification): normalize dict options (#4527) | ||
|
阿泽
|
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> |
||
|
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. |
||
|
黄云龙
|
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
|
||
|
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>
|
||
|
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 |
||
|
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 |
||
|
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.
|
||
|
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>
|
||
|
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 |
||
|
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> |
||
|
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 |
||
|
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> |
||
|
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 |
||
|
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. |
||
|
Daoyuan Li
|
c4b651fb9d
|
Fix lost loop_capped stop reason when a subagent's run_id is None (#4059)
LoopDetectionMiddleware._get_run_id used a truthiness check that collapsed a present-but-None run_id to the same "default" key as a totally absent one. SubagentExecutor sets context["run_id"] = self.run_id unconditionally, so run_id is genuinely None for an embedded/TUI-dispatched subagent, and later reads the stop reason back with that same raw attribute via consume_stop_reason(self.run_id). The write (keyed "default") and the read (keyed None) disagreed, so a genuine loop-detection hard-stop's loop_capped reason was silently dropped instead of reaching the lead. Align _get_run_id with TokenBudgetMiddleware's key-presence-based version, which does not have this bug: return the context value as-is when the key is present (None included), and fall back to a per-runtime-unique key only when the key is absent. |
||
|
Daoyuan Li
|
5edc7a889e
|
fix(security): neutralize prompt-injection tags in web_capture results (#4099)
The remote-content allowlist in ToolResultSanitizationMiddleware (`_REMOTE_CONTENT_TOOL_NAMES`) covered web_fetch / web_search / image_search but not web_capture, which was added later. The Browserless web_capture tool embeds the target site's `X-Response-Status` reason phrase — free-form text controlled by whatever server is being captured (RFC 7230 §3.1.2) — into its result message via `_target_status_warning`. A malicious page could therefore forge a `<system-reminder>` block (or a `--- END USER INPUT ---` boundary marker) through web_capture that would be escaped for web_fetch, letting attacker-influenced remote content reach the model as authoritative framework context. Add "web_capture" to the allowlist so its result is structurally neutralized for parity with the other remote-content tools. This extends the same defense introduced in #4002 to the one built-in remote-content tool it did not yet cover. Add regression tests that build the web_capture result the way community/browserless/tools.py does (real `_target_status_warning` + `BrowserlessScreenshotResult`) and assert the forged tags/boundary markers are escaped, while a benign status warning is preserved unchanged. |
||
|
Aari
|
ca18cf0b24
|
fix(agent): reserve ellipsis room so the local title respects max_chars (#4052)
`_fallback_title` sliced the user message to `min(max_chars, 50)` and then
appended a three-character ellipsis, so the returned title could be three
characters longer than the configured cap. `_parse_title`, six lines above,
slices the model's answer to `max_chars` exactly -- both read the same
`TitleConfig.max_chars`, only one honoured it.
This is the default path, not an error branch: `config.example.yaml` ships
`title.model_name: null` ("null = fast local fallback"), so every title is
produced here unless the operator opts into a title model. `max_chars` is a
documented key with a pydantic range of 10..200; any value in 10..52 makes a
long first message overshoot its cap.
Reserve room for the ellipsis before slicing. At the shipped `max_chars: 60`
the body is still 50 characters, so default output is unchanged.
The existing `test_sync_generate_title_respects_fallback_truncation` asserted
the shape of the truncation but never its length -- at its own `max_chars=50`
it was passing on a 53-character title. It now asserts the bound it is named
after.
|
||
|
Huixin615
|
6389fc03fd
|
fix: ensure visible response after tool runs (#4033)
* fix: ensure visible response after tool runs * fix: clean up terminal response recovery state * fix: bound terminal recovery state |
||
|
Nan Gao
|
8fbf101de3
|
fix(subagents): inject durable context before compaction (#4040)
* fix(subagents): inject durable context before compaction * fix(subagents): coalesce system messages after durable-context injection Address #4040 review: - append SystemMessageCoalescingMiddleware innermost on the subagent chain so the SystemMessage(authority) DurableContextMiddleware injects is merged into one leading system_message; otherwise the durable fix trades #4039's assistant-first 400 for a duplicate-system 400 on strict backends - add a two-system regression guard driving the real builder output through a strict model; assert exactly one leading SystemMessage - assert single-leading-system in the compaction integration test too - update the middleware count/last-element assertion (coalescer is now unconditionally last, removing the summarization-dependence ambiguity) - compare _skills_root against posixpath.normpath(container_path) - document the coalescer on the subagent chain in backend/AGENTS.md |
||
|
Daoyuan Li
|
79611673d7
|
Fix circuit breaker wedging after a non-retriable half-open probe (#3991)
* Fix circuit breaker wedging after a non-retriable half-open probe When the circuit breaker is half-open it admits a single probe call by setting `_circuit_probe_in_flight = True`. If that probe raised a *non-retriable* error (e.g. quota/auth), the except block skipped both `_record_failure()` (correct - business errors must not trip the breaker) and any probe reset, so the circuit stayed `half_open` with `_circuit_probe_in_flight = True` permanently. Every later call then fast-failed in `_check_circuit()` forever, because no call could run the handler to reach `_record_success` / `_record_failure`. Release the probe on the non-retriable path (mirroring the existing GraphBubbleUp handler) so the next call admits a fresh probe. The breaker still never trips on non-retriable errors. Applied to both the sync and async paths. Adds sync + async regression tests asserting the probe is released and the next `_check_circuit()` re-admits a probe. * Address review: extract _release_half_open_probe helper |
||
|
Daoyuan Li
|
1fa91fa39d
|
Fix AttributeError in ThreadDataMiddleware when runtime.context is None (#3989)
`before_agent` guards `context = runtime.context or {}` at the top, but the
`run_id` stamp on a trailing HumanMessage still read the raw `runtime.context`,
so a None context (thread_id resolved from `config.configurable`) plus a
HumanMessage last message raised `AttributeError: 'NoneType' object has no
attribute 'get'`. Use the guarded local `context` instead.
Adds a regression test.
|
||
|
hataa
|
266883b3dd
|
fix(subagents): inherit summarization middleware and harden step capture (#3875 Phase 3) (#4009)
Phase 3 of #3875 — subagents previously inherited none of the lead's context-compaction, so a deep-research subagent (max_turns up to 150) could accumulate >1M cumulative input before max_turns/timeout/token_budget engaged, even after Phase 2's budget capped the pathological tail. - Gate the subagent runtime chain on the SAME ``app_config.summarization.enabled`` switch the lead reads (per maintainer guidance in #3875), via the shared ``create_summarization_middleware`` factory. One config covers both chains; no separate ``subagents.summarization`` field. No-op when summarization is off (factory returns None). - ``skip_memory_flush=True`` on the subagent path: the factory otherwise attaches ``memory_flush_hook`` (when memory.enabled), which flushes pre-compaction messages into durable memory keyed by thread_id. Subagents share the parent's thread_id, so without skipping the hook a subagent's internal turns would pollute the PARENT thread's durable memory (#3875 Phase 3 review point). - Harden ``capture_new_step_messages`` to tolerate history contraction: summarization rewrites the messages channel via ``RemoveMessage(id=REMOVE_ALL_MESSAGES)``, shrinking len(messages) below the step-capture cursor. Without a reset, every step appended after the compaction point was dropped until length overtook the stale cursor (#3845 interaction, maintainer validation point (a)). Cursor now resets to the new tail; id/content dedup prevents re-emitting pre-compaction steps. - Couple the DEFAULT token-budget ceiling to ``summarization.enabled`` (#3875 Phase 3 review point): 1M when compaction is on, 2M when off (preserves Phase 2's deliberate headroom for summarization-off deep-research runs that can exceed 1M). A user-set budget (global or per-agent) always wins regardless of the switch. Flagged tunable. The summarization middleware does not implement ``consume_stop_reason``, so the Phase 2 guard-cap stop-reason channel is unaffected. Refs: https://github.com/bytedance/deer-flow/issues/3875 |
||
|
Aari
|
3be3969f8f
|
fix(agent): snap tool-output tail forward so fallback truncation respects max_chars (#4017)
* fix(agent): snap tool-output tail forward so fallback truncation respects max_chars _snap_to_line_boundary() moves an offset backwards to the preceding newline. That is correct for the head's end offset, but _build_fallback() also applied it to the tail's start offset, where moving backwards lengthens the tail. On output whose last newline sits in the second half (log lines followed by one long unbroken line: minified JSON, base64 artifacts), the returned string exceeded max_chars by up to 17x, defeating the guard that exists to stop a single large tool result from blowing the model context. Add _snap_start_to_line_boundary(), the forward-snapping mirror, and use it for the tail offset. The existing head_end guard becomes redundant because a forward snap can only move the tail start away from the head. test_result_never_exceeds_max_chars already asserted this invariant but passed newline-free content, so the snapping branch was never exercised. * test(agent): pin the forward-snap direction of the fallback tail The bound test only asserts that the result fits max_chars. Its content has no newline inside the tail's snap window, so _snap_start_to_line_boundary returns pos unchanged and the test stays green even with the snap removed. Place a newline in the window so the snap has to fire: the tail must begin after it, which the pre-fix backward snap does not do. |
||
|
Ryker_Feng
|
ebc09ce130
|
feat(mcp): auto-promote deferred MCP tools from routing hints (#4019)
* feat(mcp): auto-promote deferred MCP tools from routing hints
When tool_search.enabled=true defers MCP tool schemas, PR1 routing hints
still require the model to spend a tool_search discovery round trip before
it can call the tool the routing metadata already points at. This adds a
McpRoutingMiddleware that matches the latest user message against PR1
routing keywords and promotes the matching deferred schemas before the
model call, removing that round trip.
Design (soft routing, opt-in, additive):
- Matches only the latest real HumanMessage (shared is_real_user_message
helper, reused by SkillActivationMiddleware so the two cannot drift);
case-insensitive substring match, no tokenizer dependency.
- Ordering: priority desc, then tool name asc; capped by the new global
tool_search.auto_promote_top_k (default 3, clamped 1..5). Does not add or
consume a per-tool auto_promote_top_k (PR1 schema unchanged); a per-tool
value is ignored with a DEBUG note.
- Returns a plain {"promoted": ...} state update (not a Command) and relies
on ThreadState.merge_promoted for union/dedupe, so auto-promote and a
model-triggered tool_search converge on the same catalog hash.
- Installed before DeferredToolFilterMiddleware on every deferred-tool path
(lead agent, subagent, embedded client, webhook via shared builders);
a construction-time assert rejects the reversed order. catalog_hash is
None / no routing index is a complete no-op, so bootstrap and ACP skip it.
- Privacy: never executes tools, never promotes policy-filtered tools, adds
no routing keywords or matched tool names to trace metadata or INFO/WARN
logs.
No behavior change when tool_search.enabled=false.
Tests: index construction, matching semantics, middleware state updates,
same-cycle deferred-filter interaction, lead/subagent/embedded-client
builder wiring + order invariant, config clamping, config.example.yaml
parseability, and privacy assertions.
* refactor(mcp): address auto-promote review nits
- executor: access app_config.tool_search.auto_promote_top_k directly to match
the lead-agent and embedded-client paths (drop the over-defensive getattr that
masked missing config); update the subagent test mock to carry tool_search.
- tool_search / mcp_routing_middleware: cross-reference the duplicated routing
priority/keyword normalization between the builder and the middleware's
defensive _normalize_index so they cannot silently drift.
- MCP_SERVER.md: document that auto-promote keyword matching is a case-insensitive
substring test (not word-boundary), advising distinctive keywords.
|
||
|
Huixin615
|
c30c8ef759
|
fix: recover from empty tool call names (#4008)
* fix: recover from empty tool call names * test: harden empty tool call recovery --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Yi Deng
|
b36d7194d9
|
fix(security): neutralize prompt-injection tags in remote tool results (#4002)
* fix(security): neutralize prompt-injection tags in remote tool results
User input is already neutralized for framework/injection tags, but tool
results are not. Remote content fetched by web_fetch/web_search is equally
untrusted and can carry a forged <system-reminder> block that reaches the
model verbatim as authoritative context.
Extract a shared neutralize_untrusted_tags() primitive from
InputSanitizationMiddleware and apply it to remote-content tool results
(web_fetch/web_search/image_search) via a new ToolResultSanitizationMiddleware.
Local tool output (bash/read_file) is left untouched so legitimate code/file
content is never mangled.
* test: update subagent middleware count for tool-result sanitizer
The new ToolResultSanitizationMiddleware adds one entry to the shared runtime
chain (11 -> 12). Update the subagent count assertion, use a lazy import for
neutralize_untrusted_tags so the module loads even when tests stub the
input-sanitization module, and document the new middleware in AGENTS.md.
* fix(security): address review — sanitize bare str list items; document MCP scope
- Neutralize bare str elements inside a ToolMessage content list (previously
only {type:text} dict blocks were rewritten), matching the str-in-list shape
ToolOutputBudgetMiddleware._message_text already anticipates.
- Document the name-based allowlist limitation: MCP remote-content tools
registered under arbitrary names (e.g. fetch_url) are not covered; a name
heuristic is avoided to prevent mangling local tool output, with metadata
tagging tracked as a follow-up. Add a regression test pinning this boundary.
|
||
|
hataa
|
c9fb9768d4
|
fix(subagents): unify guardrail caps on additive stop_reason + add token_budget (#3875 Phase 2) (#3980)
Phase 2 of #3875. Two guardrail axes can end a subagent run early — the turn budget (GraphRecursionError) and the token budget (TokenBudgetMiddleware) — and both now surface *why* through one additive `subagent_stop_reason` field instead of a status enum. This completes and course-corrects Phase 1 (#3949), which shipped the turn-budget cap as a `max_turns_reached` status enum. The agreed Phase 2 design replaces that enum with an optional `stop_reason` field (token_capped | turn_capped | loop_capped): a new enum value would break v1 consumers, while an additive field is ignored by older frontends and ledger readers. `max_turns_reached` and SubagentStatus.MAX_TURNS_REACHED are removed. - subagents.token_budget config (default enabled, 2,000,000 tokens, warn 0.7) with per-agent override; TokenBudgetMiddleware is now attached in build_subagent_runtime_middlewares so the cost-ceiling backstop engages for every subagent. The hard-stop does not raise — it strips tool_calls and lets the run finish with a final answer, recording the cap on a per-run consume_stop_reason() accessor. - executor.py: on normal completion it reads consume_stop_reason() and stamps completed + token_capped when the budget fired; on GraphRecursionError it recovers the last AIMessage partial (completed + turn_capped) or, if nothing usable survived, failed + turn_capped. SubagentResult gains stop_reason. - status_contract.py / contracts/subagent_status_contract.json (v2) / frontend subtask-result.ts: additive subagent_stop_reason field, pinned by test_status_values_match_contract / test_stop_reason_values_match_contract. - task_tool.py + delegation_ledger.py: drop the max_turns_reached paths; the ledger captures stop_reason and renders model-facing "capped" guidance so the lead reuses a capped completion knowingly. The 2,000,000-token default is deliberately loose (tighten to taste) — it would have roughly halved the reported 4.4M burn while leaving legitimate deep-research runs (max_turns=150) room. Subagent summarization is a follow-up. |
||
|
Ryker_Feng
|
26d7a5970d
|
feat: add manual context compaction (#3969)
* feat: add manual context compaction * fix: harden manual context compaction |
||
|
AnoobFeng
|
47b0f604f4
|
feat(frontend):enhance the ask_clarification interaction with visualized card (#3956)
* feat(frontend): add structured human input cards for ask_clarification Implement a reusable Human Input Card flow for ask_clarification while keeping the existing text fallback for older clients and IM channels. Backend: - Add structured ToolMessage.artifact.human_input payloads for clarification requests. - Preserve ToolMessage.content as the readable Markdown/text fallback. - Normalize clarification options from native lists, JSON strings, plain strings, mixed scalar values, None, and missing options. - Derive input_mode as choice_with_other when options exist, otherwise free_text. - Keep disable_clarification non-interactive behavior as a plain ToolMessage with no human_input artifact. - Cover artifact persistence and Gateway message metadata preservation in tests. Frontend: - Add human input protocol types, runtime guards, extractors, response builders, and thread-state helpers. - Add reusable HumanInputCard with option buttons, free-text input, pending, read-only, disabled, and answered states. - Render structured clarification cards from artifact.human_input, with Markdown fallback for malformed or legacy tool messages. - Preserve line breaks in structured question/context/option text. - Hide submitted clarification bridge messages from the chat UI via additional_kwargs.hide_from_ui. - Send structured human_input_response metadata through the fourth sendMessage options argument, preserving run context in the third argument. - Wire submissions for normal chats, custom agent chats, agent bootstrap chats, and sidecar chats. - Derive answered state from raw thread.messages so hidden replies still update the original card. - Clear pending state when the hidden reply arrives, dispatch is dropped, or a later async stream failure appears on thread.error. * perf(frontend): optimize HumanInputCard UI interactions - Support Enter key to submit text input (Shift+Enter for newline) - Render question and context fields as Markdown instead of plain text - Replace deprecated FormEventHandler type with structural typing * test(frontend): add unit test cover optimize HumanInputCard UI interactions * feat(frontend): disabled chatbox when has new human-input-card * fix(style): lint error fix * fix: sanitize hidden human input replies - Preserve IME composition safety for human input card Enter submits - Treat hidden human input responses as genuine user messages for sanitization - Keep hidden card replies in memory filtering while excluding malformed/internal hidden messages - Add regression coverage for card IME handling and hidden reply sanitization * fix: tighten human input response validation - Reject empty hidden human input response values - Remove invalid list ARIA role from human input card options - Add backend coverage for empty response payloads --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
He Wang
|
fd41fdb065
|
feat(middleware): add structured tool result meta and tool-progress state machine (#3601)
* feat(middleware): add structured tool result meta and tool-progress state machine
feat:
- Add tool_result_meta.py: ToolResultMeta dataclass (status/error_type/retryable/
recoverable_by_model/recommended_next_action/source) + normalize_tool_result and
stamp_exception_meta utilities; classifies every ToolMessage regardless of path
- Add ToolProgressMiddleware: per-(thread_id, tool_name) state machine ACTIVE →
WARNED (hint injected as HumanMessage) → BLOCKED (call short-circuited); Jaccard
near-duplicate detection for repeated successful results; auth/config/internal
errors bypass WARNED and go directly to BLOCKED; LRU-bounded thread state store
- Add ToolProgressConfig: all thresholds configurable (stagnation_threshold,
warn_escalation_count, jaccard_similarity_threshold, exempt_tools, etc.);
disabled by default (enabled: false)
- Wire ToolProgressMiddleware as outer wrapper around ToolErrorHandlingMiddleware
in _build_runtime_middlewares so it receives results already carrying
deerflow_tool_meta
fix:
- ToolErrorHandlingMiddleware now calls stamp_exception_meta on exception path and
normalize_tool_result on success path so every ToolMessage carries deerflow_tool_meta
test:
- Add test_tool_result_meta.py: 26 cases covering all classification paths,
stamp_exception_meta, and normalize_tool_result Command passthrough
- Add test_tool_progress_middleware.py: 27 cases including full async paths,
Jaccard duplicate detection, LRU eviction, hint injection, and malformed meta
passthrough
- Extend test_tool_error_handling_middleware.py: middleware ordering invariant and
meta stamping on exception
docs:
- Add tool_progress section to config.example.yaml with all fields and descriptions
- Update CLAUDE.md middleware chain documentation (entries 8-9)
Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
* fix(middleware): recoverable errors stay WARNED; fix auth keyword shadowing
fix:
- WARNED is terminal for recoverable_by_model=True errors (no_results, not_found,
permission); hint re-injected on each problem call instead of escalating to
BLOCKED, so the model can retry with different parameters (e.g. fresh query,
new URL) without being hard-blocked by a prior stagnation count.
Non-recoverable (rate_limited, transient) still escalate WARNED → BLOCKED
after warn_escalation_count more problems; auth/config/internal remain
immediately BLOCKED.
- Remove bare "api key" keyword from auth classification rule so "no api key
configured" correctly classifies as config (not auth), producing the accurate
block-reason text for the model.
docs:
- CLAUDE.md: document all three ToolProgressMiddleware transition paths
- config.example.yaml: update inline state-machine comment to match new paths
test:
- test_recoverable_errors_stay_warned_indefinitely: WARNED never escalates for
recoverable errors regardless of how many problem calls accumulate
- test_recoverable_error_re_injects_hint_past_escalation: hints continue past
the escalation zone for recoverable errors
- test_no_api_key_is_config_not_auth: regression guard for keyword shadowing fix
Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
* fix(tool_result_meta): add JSON error extraction and fix source classification
fix:
- Fix non-standard error path: source was "exception" but should be "tool_return"
- Add _extract_json_error_text to isolate JSON error fields from noisy JSON bodies
(e.g. Brave Search {"error": "...", "query": "..."} — query keywords no longer
pollute error classification)
- Add success-path JSON extraction to catch tools that return HTTP 200 with a JSON
error body (status="success" but {"error": "API key not configured"})
- Add _SEMANTIC_ZERO_ERROR_STRINGS frozenset to suppress false positives from tools
that use {"error": "none"} / {"error": "null"} / {"error": "ok"} as success signals
- Document that stamp_exception_meta always overwrites existing TOOL_META_KEY
(exception-derived classification is authoritative over tool return-time stamps)
test:
- Add parametrized regression tests for all semantic-zero error strings
- Add tests for non-standard error path source field
- Add tests for JSON error extraction (nonstd, success-path, numeric, falsy values)
- Correct test comment for test_no_api_key_is_config_not_auth
Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
* fix(tool_progress_middleware): fix 6 bugs, add terminal guard and structured logging
fix:
- H1: fix exempt_tools empty-set silently ignored — use `is not None` instead of
truthiness check so ToolProgressConfig(exempt_tools=set()) correctly disables all
exemptions
- Fix _get_block_reason creating phantom LRU entries via _get_state (write path);
now uses dict.get + explicit move_to_end on read path only
- Fix _pending memory leak: LRU eviction of _phase_states now synchronously removes
all (evicted_thread, *) keys from _pending
- Fix _assess_and_transition missing terminal guard for blocked state — a recoverable
error result could silently demote blocked → warned in concurrent-race scenarios;
early return preserves terminal semantics
- Fix recent_word_sets window: stored [-5:] but is_near_duplicate only compared [-3:];
align to [-3:] and change type list→tuple (prevents accidental in-place mutation
across dataclasses.replace shallow copies)
- Fix _format_hint missing "success" key and "continue" action: Jaccard near-duplicate
results produced the generic fallback instead of a specific actionable message
feat:
- Add structured state-transition logging (ACTIVE/WARNED/BLOCKED transitions, blocked
intercepts, hint injection debug log)
test:
- Add regression tests for all 6 bug fixes (H1, phantom LRU, pending leak, terminal
guard, window alignment, format_hint near-dup)
- Add Jaccard near-threshold boundary test (7/9 vs 8/9 Jaccard)
- Add production min_words=10 skip test for short content
- Add exempt_tools empty-set and None round-trip tests
- Add _augment_request deduplication test
- Add before_agent current-run preservation test
- Add structured logging tests (WARNED/BLOCKED/ACTIVE/intercepted/debug)
Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
* chore(config): remove unused backward-compat fields from ToolProgressConfig
Remove max_calls_per_intent and window_size fields that were marked
"Retained for backward compatibility; not used by the current state machine"
when the state machine was introduced. Pydantic v2 ignores unknown fields
by default, so existing config.yaml files with these keys remain valid.
Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
* fix(tool_progress): address PR review and multi-agent review findings
fix:
- Remove <80-char length gate for partial_success; only _PARTIAL_MARKERS now
- Add word-boundary regex for numeric HTTP codes (401/403/404/500) to avoid
false positives like "500ms" or "4010 rows" triggering hard-block
- Add "task" to default exempt_tools (delegation primitive, not a search tool)
- Remove move_to_end() from _get_block_reason read path; blocked threads were
permanently warm in LRU, starving active threads of eviction slots
- Add _reset_blocked_states in before_agent: scope BLOCKED and WARNED states
to a single run; clear recent_word_sets so stale Jaccard windows don't cause
false near-duplicate detections in the next run
- Compute word_set() lazily (only for success results); cap content at 8192
chars to bound memory and CPU cost on large tool results
- Remove unused retryable field from ToolResultMeta (no consumer existed)
- Add isinstance-based ordering guard and warning log for missing meta
- Fix JSON-without-error-key fallback: use _UNKNOWN_ERROR instead of
classifying incidental field values (e.g. {"user_id": 401} → auth → stop)
- Fix _extract_json_error_text: use json.dumps for dict/list error fields
instead of str() which produced Python repr matching config rules spuriously
- Add "no results found"/"no content found"/"no images found" to _PARTIAL_MARKERS
so success responses with empty results trigger stagnation detection
- Fix immediate-block path to increment consecutive_problems (was left at 0)
- Fix _queue_assessment: skip phantom _pending entries for evicted threads
- Bump config_version 13→16 (upstream added 14/15; tool_progress is additive)
test:
- Update test_short_content_is_partial → test_short_terse_success_is_not_partial
- Add parametrized test_numeric_keyword_word_boundary (8 positive + negative cases)
- Add test_before_agent_resets_blocked_states_for_new_run (strengthened assertions)
- Add test_before_agent_resets_warned_states_for_new_run
- Add test_missing_meta_on_non_exempt_tool_emits_warning
- Add test_middleware_ordering_guard_raises_when_progress_is_inner
- Add test_auth_error_immediately_blocked asserts consecutive_problems == 1
- Add tests for JSON-without-error-key, dict error field, no-results partial_success
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(tool_progress): address second PR review — perf, architecture doc, concurrency note
fix:
- Extract content.lower() once before _PARTIAL_MARKERS check in normalize_tool_message;
previously computed up to 7× per call inside the generator (once per marker)
docs:
- Add division-of-labor paragraph to ToolProgressMiddleware module docstring explaining
coexistence with LoopDetectionMiddleware: result-quality guard (per-tool BLOCK) vs
call-pattern guard (whole-turn hard-stop); no shared state, no double-stop risk
- Add threading.Lock comment explaining why asyncio.Lock is not used (short critical
sections, must also protect sync wrap_tool_call path from subagent executor threads)
- Update backend/CLAUDE.md entry 8 with division-of-labor summary; fix entry 9
(remove stale retryable field reference, add missing recoverable_by_model/source)
test:
- Add test_tool_progress_and_loop_detection_coexist_without_interfering: drives both
middlewares to WARNED state simultaneously, verifies independent state, independent
hint queues, and no cross-contamination; uses snapshot copy for final assertion
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(tool_progress): reset all tool states at run boundary; fix semantic-zero test validity
fix:
- _reset_run_states (formerly _reset_blocked_states) drops the phase filter and
resets all tracked (thread, tool) pairs unconditionally at before_agent; ACTIVE
tools with sub-threshold consecutive_problems or cached recent_word_sets no longer
bleed into the next run, preventing spurious WARNED transitions on clean R2 calls
- test_normalize_json_semantic_zero_error_string_not_treated_as_error: replace
{error_value!r} f-string (produces invalid JSON with single quotes) with
json.dumps so _extract_json_error_text actually parses the payload and the
_SEMANTIC_ZERO_ERROR_STRINGS guard is exercised, not bypassed at json.loads
test:
- add test_before_agent_resets_active_state_consecutive_problems_and_word_sets to
lock the ACTIVE-phase run-boundary reset: drives tool to active/cp=1/ws≠() in R1,
asserts both fields are zero/empty after before_agent fires for R2
* docs(tool_progress): document intentional per-run reset vs LoopDetection thread-scoped retention
Addresses reviewer observation in PR #3601 that _reset_run_states diverges
from LoopDetectionMiddleware's cross-run scoping policy without explanation.
Expands the _reset_run_states docstring to record the intentional design
choice: ToolProgressMiddleware resets per-run because result-quality errors
(rate_limited, transient) are time-bound and may resolve between turns —
retaining stale counters would risk false-positive BLOCKED calls.
LoopDetectionMiddleware retains history across runs because call-pattern
loops are time-invariant. The divergence is by design, not oversight.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(middleware): restore ReadBeforeWriteMiddleware as outermost write gate
A merge conflict resolution had accidentally placed ReadBeforeWriteMiddleware
after ToolErrorHandlingMiddleware (inner), reversing the original intent from
b81334cc where it was the outermost write gate before ToolErrorHandling.
fix:
- Restore ReadBeforeWriteMiddleware to outer position: ReadBeforeWrite →
ToolProgress → ToolErrorHandling. Blocked writes now return immediately
without consuming a ToolProgress slot.
- Add normalize_tool_result call on blocked ToolMessages so they carry
deerflow_tool_meta (recoverable_by_model=True) even though they bypass
ToolErrorHandlingMiddleware.
test:
- Add test_blocked_write_has_deerflow_tool_meta (sync + async) to lock the
normalize_tool_result behavior on blocked writes.
- Fix chain order assertions in TestChainWiring and
test_build_lead_runtime_middlewares_chain_order_matches_agents_md.
docs:
- Renumber AGENTS.md items: 10→ReadBeforeWrite, 11→ToolProgress,
12→ToolErrorHandling; update descriptions to reflect outermost-gate design.
- Fix stale cross-reference: LoopDetectionMiddleware (item 23) → (item 25).
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4 <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
|