mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-07-25 15:37:53 +00:00
2775 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Daoyuan Li
|
2a7469cdbc
|
fix(channels): dedupe GitHub webhook redeliveries (#4104)
* fix(channels): dedupe GitHub webhook redeliveries The inbound dedupe added for the IM channels in #3584 keys ChannelManager._is_duplicate_inbound on a top-level metadata["message_id"] plus a workspace id. The GitHub channel added later in #3754 never populated either: the X-GitHub-Delivery GUID was buried in metadata["github"]["delivery_id"] and no workspace id was set, so every GitHub delivery produced a None dedupe key and was never deduped. GitHub reuses the same X-GitHub-Delivery GUID when a delivery is retried after a timeout or replayed via the repo/App "Redeliver" button, so a redelivery re-ran the agent with real side effects (e.g. a duplicate PR comment) while the other channels absorbed an identical redelivery. fanout_event now stamps the dedupe identity the manager already consumes: - workspace_id = repo (globally unique, always present; mirrors Telegram/WeChat keying the workspace on the chat id) - metadata["message_id"] = f"{delivery_id}:{agent.name}" A single delivery fans out to N agents, so the id is scoped to (delivery, agent): an identical redelivery reproduces the same pairs (deduped) while two agents matching the same delivery keep distinct ids and both still fire. When the delivery header is absent the id is left None, so the manager fails open exactly as before. Tests: dispatcher-level coverage that the identity is stamped, stable across redelivery, and distinct per agent/delivery; a ChannelManager regression that an identical GitHub redelivery dispatches once while a new delivery and a second agent on the same delivery still fire. * fix(channels): scope GitHub dedupe id by owning user _inbound_dedupe_key indexes on (channel, workspace_id, chat_id, message_id). For GitHub, workspace_id and chat_id are both the repo, so the owning user was never represented anywhere in the key - fanout_event stamped the id as f"{delivery_id}:{agent.name}". Two different users each binding an agent of the same name (e.g. "reviewer") to the same repo+event therefore produced an identical dedupe id for both fan-out messages. ChannelManager._is_duplicate_inbound treated the second as a replay of the first and silently dropped it, even though GitHub delivered the webhook once and both users' agents legitimately matched. Fold match.user_id into the id: f"{delivery_id}:{match.user_id}:{agent.name}". Genuine redeliveries (same user, same agent, same delivery) still produce the same id and are deduped; two agents - same-named or not, same user or not - on one delivery now always keep distinct ids. Tests: new cross-user regression pins that two users' same-named agents both dispatch instead of the second being deduped against the first; the existing per-(delivery, agent) test and the literal id assertion in test_delivery_id_populates_inbound_dedupe_identity are updated for the new id shape. * fix(channels): point cross-user dedupe comment at its regression test Replace the inline reviewer-handle attribution with a pointer to test_dedupe_identity_distinguishes_same_agent_name_across_users, which ages better than a person's name in committed code. |
||
|
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> |
||
|
hataa
|
1300c6d36b
|
feat(authz): add pluggable AuthorizationProvider protocol and config scaffolding (Phase 0, #4063) (#4127)
Phase 0 of the RFC in #4063: scaffolding only, zero behavior change. New deerflow/authz/ package (sibling to deerflow/guardrails/): - AuthorizationProvider Protocol: authorize() + aauthorize() + filter_resources() - Principal/AuthzRequest/AuthzDecision/AuthzReason dataclasses - GuardrailAuthorizationAdapter: bridges AuthorizationProvider → GuardrailProvider so existing GuardrailMiddleware can enforce authz decisions without a new middleware New authorization config section (default enabled: false): - AuthorizationConfig wired into AppConfig alongside guardrails - Singleton load/reset mirrors GuardrailsConfig pattern - config.example.yaml documents the RBAC provider schema 29 tests covering protocol conformance, dataclass construction, adapter request/decision mapping, and config singleton behavior. Per RFC #4063 Phase 0 (foundations). Layer 1/2 wiring and Principal builder in services.py deferred to Phase 1. |
||
|
Zheng Feng
|
f3e4e5f8f0
|
ci(helm): publish chart to charts/ namespace prefix on GHCR (#4175)
* ci(helm): publish chart to charts/ namespace prefix on GHCR
Push the Helm chart to ghcr.io/<owner>/charts/deer-flow (via a `charts/`
prefix on the `helm push` target) instead of the bare `deer-flow`
package. This namespaces the chart apart from the image packages
(deer-flow-{backend,frontend,provisioner}) without renaming the chart:
Chart.yaml `name` stays `deer-flow`, so dir = chart name = in-cluster
resource/selector names, and no `nameOverride` hack is needed.
The chart is new in 2.1.0 (chart infra landed in #3987, after v2.0.0),
so 2.1.0 is the first chart release. Early nightly builds remain at the
legacy non-prefixed ghcr.io/<owner>/deer-flow.
Also refresh chart release docs:
- Replace the removed scripts/build-and-push.sh in the chart README with
raw `docker build`/`push` commands (contexts/args match container.yaml).
- Point NOTES.txt's empty-registry warning at the README section instead
of the removed script.
- Retarget RELEASING.md version examples from 2.2.0 to 2.1.0.
- Bump the chart README's helm prerequisite to 3+.
* docs(helm): require helm 3.8+ and note legacy chart package cleanup
Address PR #4175 review:
- bump documented minimum to helm 3.8 (OCI registry support stabilized
there; earlier 3.x needs HELM_EXPERIMENTAL_OCI=1)
- add a post-release note to delete/revoke the legacy bare
ghcr.io/<owner>/deer-flow chart package after 2.1.0
|
||
|
Daoyuan Li
|
0dd90ccfde
|
fix(agents): require config.yaml in update_agent's legacy-agent guard (#4166)
update_agent (the harness tool) and PUT /api/agents/{name} (the same
operation over HTTP) share an identical guard meant to block updates to
an agent that only exists in the legacy shared layout. The guard checked
bare directory existence:
if not agent_dir.exists() and paths.agent_dir(name).exists():
When memory is enabled, the first time a user chats with a legacy shared
agent, the memory writer creates a per-user directory containing only
memory.json (no config.yaml). agent_dir.exists() is then true, so the
guard never fires: the tool falls through to load_agent_config, which
resolves through to the legacy shared config via the already-hardened
resolve_agent_dir, and silently writes a brand-new config.yaml/SOUL.md
into the memory-only directory. That forks the agent for just this user;
every other user keeps reading the original shared config forever, with
no error or warning.
resolve_agent_dir itself was already hardened against exactly this
failure mode: it requires config.yaml to exist, not just the directory.
Mirror that condition at both call sites here.
|
||
|
Aari
|
8e96a6a252
|
fix(security): html-escape the conversation block in MEMORY_UPDATE_PROMPT (#4162)
format_conversation_for_update embeds raw user turns into the <conversation> slot of MEMORY_UPDATE_PROMPT. This is the most attacker-influenced input in the prompt, and it was unescaped: a message containing "</conversation><current_memory>..." closes the conversation block and forges a <current_memory> authority section for the extraction LLM, which can be steered into persisting an arbitrary high-confidence fact — and that fact is later injected into the lead-agent system prompt's <memory> block, which the prompt declares trusted. This is the last unguarded sibling of a rule the repo has established repeatedly. #4044/#4060 html-escaped the current_memory slot of this exact template; #4097 escaped the <memory> injection renderer. In updater.py the same .format() call escapes current_memory and leaves conversation raw. The memory updater sees raw text because InputSanitizationMiddleware only rewrites the ModelRequest and never mutates state, while MemoryMiddleware queues the raw state messages. Escape content with html.escape(quote=False), mirroring _escape_summary / _format_fact_line — after truncation so a trailing "..." cannot split an entity, on both human and assistant turns. Render-time only: no stored value is mutated, so the apply path is unaffected. The conversation function already strips <uploaded_files> here, so tag hygiene in this renderer is established. Scope is the memory updater. The summarizer's <new_messages> / <existing_summary> blocks are the same rule unguarded, but their output is quarantined as untrusted durable context rather than promoted to system authority; that hardening will be a separate change. |
||
|
Daoyuan Li
|
656f6b364c
|
fix(skills): recognize fully deleted public skill packages in review CI (#4169)
select_skill_packages() resolved every changed non-SKILL.md path to its owning package via an unconditional depth-3 fallback, then queued that path for review. When a PR deletes an entire public skill package (not just SKILL.md, but its scripts/assets/etc. too), the fallback still returned the package directory even though it no longer exists on disk post-deletion. The review CLI then reported a false structure.missing-skill-md blocker for a path that isn't there, failing CI on a routine, correct package removal. Skip a resolved package only when every changed file under it was a deletion and the package directory itself is gone from disk - i.e. the whole package was intentionally removed. A package left in a broken/partial state (e.g. SKILL.md deleted while sibling files remain) still resolves to an existing directory, so it is unaffected and continues to be queued and flagged. |
||
|
qin-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. |
||
|
Daoyuan Li
|
51bb19fa1b
|
fix(channels): drop redundant GitHub review-comment webhook fan-out (#4131)
* fix(github): drop redundant pull_request_review_comment fan-out noise
GitHub fires one pull_request_review_comment webhook per inline comment
attached to a pull_request_review submission, in addition to the single
pull_request_review event for the review itself. A bot reviewer like
CodeRabbit commonly leaves 20-30 inline comments per review, flooding
the webhook with near-duplicate deliveries that carry nothing an agent
doesn't already have -- it fetches every inline comment itself via
`gh api` when it processes the parent pull_request_review event.
Filter these out in fanout_event() before the registry lookup / per-
agent loop, since the redundancy is a property of the event itself, not
of any specific agent binding. A companion comment is identified by
pull_request_review_id being set (it belongs to a review) and
in_reply_to_id being absent (it is not itself a reply within an
existing thread -- that case is a genuine new interaction and must
still fire).
* fix(channels): scope review-comment suppression to bindings that also see the review
The redundant pull_request_review_comment filter suppressed every
companion comment unconditionally, before the registry lookup even ran.
That premise only holds for a binding that also subscribes to
pull_request_review on the same repo -- events are opt-in per binding,
so a binding registered for pull_request_review_comment alone never
receives the parent review event and the companion comments were its
only delivery of the review's inline content. Suppressing those too was
a silent, total loss for that binding, not noise reduction.
Move the check into the per-agent loop so it only suppresses a matched
binding's companion comment when that same binding also has an active
pull_request_review trigger on this repo, reusing the existing registry
lookup rather than re-walking bindings by hand. Bindings subscribed to
pull_request_review_comment alone now always fire, matching the
opt-in-per-binding contract documented in triggers.py.
* fix(channels): close require_mention gap in review-comment redundancy gate
willem-bd's second review round found that the per-binding redundancy gate
(commit b3791b33) treated a binding as "covered" purely by checking whether
it also registers a `pull_request_review` trigger on this repo, ignoring
whether that trigger itself requires a mention. A `pull_request_review_comment`
payload never carries the paired review's own top-level body, so there is no
way to verify from a comment delivery whether that trigger's own
`require_mention` check would actually pass. A human `@mention` living only
in one inline comment (not the review summary) could therefore be lost
twice: the review event filtered out by its own `no_mention` gate, and the
one inline comment that carries the mention dropped here as "redundant" --
the same silent-loss shape as the original bug, through a narrower path.
The gate now also requires the paired `pull_request_review` trigger's
resolved `require_mention` to be false before treating it as coverage,
trading a small amount of residual redundancy (an extra companion delivery
when the review would have fired anyway) for zero silent loss.
While in the same code, also addressed two smaller review notes:
- The redundant-comment skip reason now prefers the companion's own trigger
verdict when that verdict is also a skip (e.g. its own `require_mention`
independently fails), instead of always reporting the generic
`redundant_review_comment` label.
- `_pr_review_prompt` now tells the agent to fetch a review's inline
comments via `gh api .../reviews/{id}/comments` -- the redundancy gate's
suppression is only genuinely redundant if the agent actually recovers
that content from the parent review event, and nothing previously told it
to (zhfeng's review).
Tests: reproduces willem-bd's exact scenario (dual-subscribed binding,
require_mention on the review trigger, mention present only in the inline
comment) and confirms it fails on the prior code and passes with the fix;
adds a multi-agent-independence test and a skip-reason-precedence test;
adds prompt tests locking the new fetch-hint text and its missing-id guard.
Fail-before/pass-after verified via patch-file revert (not stash, to avoid
colliding with sibling worktrees).
|
||
|
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> |
||
|
Daoyuan Li
|
cdefd4a8aa
|
fix(mcp): invalidate tools cache on config content + path, not just newer mtime (#4124)
* fix(mcp): invalidate tools cache on config content + path, not just newer mtime The MCP tools cache invalidated only on a strict extensions-config mtime `>` comparison and tracked no resolved config path, so `_is_cache_stale()` missed: - content changes with an unchanged mtime (same-second edits; object-store / network mounts that do not bump mtime); - content changes with a backward mtime (git checkout, cp -p / backup restore, tar / rsync preserving timestamps); - a resolved-path switch to a different config file with mtime <= the recorded value (structurally invisible — no path was tracked at all). On multi-worker (uvicorn/gunicorn) or stale-mtime deployments this leaves the LangGraph-embedded runtime and every non-writer worker serving stale MCP tools after `PUT /api/mcp/config`, breaking the module's documented promise that changes made through the Gateway API are reflected in the embedded runtime. Record the resolved config path and a `(mtime, size, sha256)` content signature at initialization and invalidate when the path OR the signature differs (`!=`), mirroring `config/app_config.py::get_app_config()` so the two runtime-editable config files share one content-based staleness signal. The per-call stat was already paid, and the small-file sha256 matches the cost app_config pays per request. The "config missing / not yet initialized" no-op behavior and the cache reset endpoint are preserved. Adds backend/tests/test_mcp_cache.py covering all three failure modes plus unchanged-file and forward-edit sanity cases. Also folds in an incidental backend/AGENTS.md doc-sync: the restart-required field list was missing `scheduler` and `run_ownership`, both present in reload_boundary.py::STARTUP_ONLY_FIELDS. * test(mcp): pin cache staleness contracts raised in review Two review observations on the content-signature cache fix, both raised as non-blocking design questions rather than bugs: - Whether relying on mtime+size alone (skipping the sha256) could ever be "optimized" back in, reopening the narrow same-second / identical-length swap gap the signature was built to close. - Whether the extensions config being deleted entirely after a successful init leaves the cache in a defined state, since current_signature flips to None and _is_cache_stale() returns False. Neither is a behavior change: both were already the intended contract, preserved verbatim from the pre-fix mtime-only code (which also returned False once the file could no longer be stat-ed). Record the reasoning inline and add regression tests that pin each contract so a future change cannot alter either silently: - test_same_mtime_same_size_swap_is_stale: a same-length server-name swap that leaves mtime AND size unchanged (the precise scenario from review, sharper than the existing same-mtime test, which also changes size) is still caught only because the sha256 is computed unconditionally. - test_config_deleted_after_init_is_not_stale: deleting the config file after init keeps the cache serving its last-known-good MCP tools instead of invalidating into an unconfigured state. Both new tests were confirmed to fail against a deliberately reintroduced version of the regression they guard (hash short-circuit / removed None-guard), then confirmed to pass against the real code. |
||
|
yym36991
|
abd0c7a585
|
docs(api): document stateless /api/runs/stream endpoint (#4177)
* docs(api): document stateless /api/runs/stream endpoint
Record that clients can start a conversation without pre-creating a thread,
and that Gateway returns thread_id and run_id via the Content-Location header.
Co-authored-by: yym36991@gmail.com
* docs(api): show on_run_created for stateless stream continuation
The Python SDK example now captures thread_id/run_id via on_run_created
before documenting the follow-up call with config.configurable.thread_id,
matching the TS/cURL examples and langgraph-sdk 0.3.x behavior.
Co-authored-by: yym36991@gmail.com
|
||
|
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> |
||
|
黄云龙
|
88a1311fe8
|
fix(DB): legacy backfill creates missing Index objects on existing tables (#4090)
* fix(DB): legacy backfill creates missing Index objects on existing tables _run_baseline_create_all_sync calls create_all(tables=..., checkfirst=True). SQLAlchemy's Table.create(checkfirst=True) skips the table AND all its Index objects when the table already exists, so an index added to the ORM model after the table was first provisioned (e.g. the partial unique index uq_channel_connection_active_identity on channel_connections) is never created, and stamping 0001_baseline skips alembic's own create_index call too. Fix: after create_all, explicitly create every Index on every baseline table with Index.create(checkfirst=True), which checks for the specific index independently of the table. Co-Authored-By: Claude <noreply@anthropic.com> * docs(bootstrap): note future-index backfill collision risk for revisions Add a short forward-looking block under the legacy index-level backfill (at backend/packages/harness/deerflow/persistence/bootstrap.py:308-322) pointing the next contributor at the same shape risk the module already documents for _baseline_TABLE_NAMES: if a future post-baseline revision adds an index to a baseline table via op.create_index(...) without checkfirst=True, the backfill above will already have pre-created it and the upgrade will collide with 'index already exists'. Mirror safe_add_column and use checkfirst=True (or a future safe_create_index helper) in such a revision. * fix(#4090): scope legacy backfill index loop to _BASELINE_INDEX_NAMES willem-bd identified that the index loop iterated table.indexes (current ORM models full set), which includes post-baseline indexes like uq_runs_thread_active from 0004. Creating these prematurely before their owning revisions dedup step raises IntegrityError on legacy DBs with duplicate active rows -- bricking bootstrap. Changes: - Add _BASELINE_INDEX_NAMES: frozenset of 23 indexes that 0001_baseline actually creates (mirrors _BASELINE_TABLE_NAMES pattern) - Guard index-creation loop with _BASELINE_INDEX_NAMES filter - Wrap each idx.create() in try/except for graceful duplicate-data handling (uq_channel_connection_active_identity has no owning revision dedup) - Fix misleading checkfirst=True comment for alembic op.create_index - Add guard test pinning _BASELINE_INDEX_NAMES against 0001 output - Add regression test with duplicate active runs (the exact crash case) - Add regression test with duplicate channel connections (graceful handling) * fix(tests): update legacy backfill fixtures to current schema - Seed runs and channel_connections using the actual 0001 baseline columns. - Preserve the duplicate-active-row conditions exercised by the regression tests. - Apply ruff formatting required by backend CI. --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Vanzeren
|
13fd8e229a
|
fix(checkpoint): persist run duration in checkpoints for history reads (#4118)
* fix: persist run duration in checkpoints for history reads * fix(checkpoint): harden run duration persistence * fix(checkpoint): persist run durations in metadata * fix(checkpoint): address review findings for run duration persistence - Add valid_duration_entry() shared validation helper (worker.py) - Rename _persist_run_durations -> persist_run_durations as public API - Import public persist_run_durations and valid_duration_entry in threads.py - Use BackgroundTasks for lazy backfill write to avoid blocking history reads - Add TODO about O(runs) growth of run_durations in checkpoint metadata - Document REGENERATE_HISTORY_RAW_SCAN_LIMIT doubling assumption * fix(checkpoint): replace pruning TODO with justification Accumulated run_durations overhead (~50 bytes/run_id) is negligible compared to messages channel blobs; no pruning strategy is needed. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
pclin
|
fabadae416
|
add Volcengine Coding Plan to quikly setup (#4141)
* add Volcengine Coding Plan to quikly setup * modify model list |
||
|
Willem Jiang
|
192ea5155e
|
Revert "fix(firecrawl): align tools with installed firecrawl-py v2 SDK surfac…" (#4165)
This reverts commit b7b4b49bfea18171cf7ba5091ffd1f9a52dd43e8. |
||
|
黄云龙
|
b7b4b49bfe
|
fix(firecrawl): align tools with installed firecrawl-py v2 SDK surface (#4151)
Replace FirecrawlApp with Firecrawl (v2 unified client) so the map/crawl/interact/extract tools target methods that actually exist: - map_url -> map (with sitemap kwarg instead of ignore_sitemap) - crawl_url -> crawl (keyword args instead of nested params dict) - interact -> scrape+actions (structured action dicts, not NL string) The installed firecrawl-py==4.23.0 has no map_url/crawl_url methods and interact(job_id, code=) does not accept url/actions params. All three tools previously deterministically returned Error:... before making a valid request. Also drop unused Optional import (ruff UP045). Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Huixin615
|
24648194ae
|
fix(frontend): show branch action only for completed turns (#4147)
* fix(frontend): gate branch action by completed turn * test(frontend): clarify branch turn boundaries |
||
|
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 |
||
|
Aari
|
81b3ed0188
|
fix(skillscan): recognize remaining outbound network sinks (#4153)
_call_is_network_sink missed the HEAD/OPTIONS verbs on requests/httpx, socket.create_connection, and urllib.request.urlretrieve. A bulk env dump or reverse shell shipped through any of these slipped past the CRITICAL exfil/reverse-shell rules whenever the URL was assembled at runtime (the non-literal case the string-literal URL check can't cover). Also treat socket.create_connection as the socket primitive in the reverse-shell shape. http.client.HTTP(S)Connection is intentionally left out: only the lazy constructor is statically visible (the request()/connect() that performs the I/O is an instance method the call-name analyzer can't resolve), so flagging the constructor would hard-block benign code that only builds a connection object. Cover the alias-resolved forms too: the sink check runs on the name after from-import / import-as resolution, a path the suite exercised only on the env-read side (#4087) and not on the sink side. |
||
|
Aari
|
79cdd99fca
|
fix(mcp): validate MCP tool names at load so deferred prompts stay inert (#4154)
* fix(tools): escape MCP tool names rendered into deferred prompts get_deferred_tools_prompt_section and get_mcp_routing_hints_prompt_section list MCP tool names into the <available-deferred-tools> and <mcp_routing_hints> system-prompt blocks without escaping, while the mirror get_skill_index_prompt_section (per its docstring) does escape. An MCP name is taken verbatim from an external server, so a crafted name could close the block and forge a framework tag. Escape names (and routing keywords) at render, mirroring the skill-index section. * fix(mcp): validate tool names at the load boundary Escaping at render only neutralizes < > &, so a tool name with newlines or markdown still injects free-form text into the deferred-tools prompt block. Deferred (tool_search) tools are never bound, so the provider's function-name check never runs on them. Drop any MCP tool whose name is not a valid identifier (^[A-Za-z0-9_-]+$) in get_mcp_tools() — the same charset the provider enforces at bind time — before it can enter the catalog or the prompt. Render-time html.escape stays as defense-in-depth. Mirrors the load-time skill-name validation in skills/storage/skill_storage.py. |
||
|
Aari
|
e2816eaa97
|
fix(models): scope the OpenAI-compat rules to BaseChatOpenAI, not a class-path allowlist (#4146)
* fix(models): scope the OpenAI-compat rules to BaseChatOpenAI, not a class-path allowlist * address review: drop redundant stream_usage helper, close test matrix - Remove _enable_stream_usage_by_default and its now-unused _OPENAI_COMPAT_USE_PATHS tuple. The class-field stream_usage fallback already sets stream_usage=True for every BaseChatOpenAI subclass (they all declare the field), so the helper's use-path allowlist gated nothing real — verified a no-op in prod, and the two stream_usage tests stay green on main with the helper present. Those tests used a BaseChatModel stub that does not declare the field; point them at a real ChatOpenAI capturing class so they exercise the fallback they now depend on. - Give the non-OpenAI normalization-skip test an actual api_base value so it exercises the skip path (api_base passed through verbatim, never rewritten to base_url). - Add an Unreleased CHANGELOG entry for the api_base behavior change on the five affected subclasses. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Mohammed Ansari
|
4af6178358
|
feat(trace): add agent observability with Monocle (#4024)
* Add Monocle tracing Enable Monocle (OpenTelemetry tracing for LLM apps) with one setup call plus the monocle_apptrace dependency. setup_monocle_telemetry auto-instruments the frameworks already in use and writes traces to .monocle/. Additive; no changes to application logic. * Config-gate Monocle telemetry in the Gateway lifespan Addresses review on #4024: moves setup_monocle_telemetry out of agents/__init__ import time into the Gateway lifespan, gated by MonocleTracingConfig (MONOCLE_TRACING env, default off). Warns on the Langfuse/global-OTel-provider conflict and relies on monocle_apptrace's own duplicate-setup guard and existing-provider attach. Pins monocle_apptrace>=0.8.8 (+ uv.lock), adds .monocle/ to .gitignore, adds tests (default-off / toggle-on / no import-time setup), and documents exporters, Okahu, and the VS Code viewer in README, config.example.yaml, and backend/AGENTS.md. * Clarify Monocle/Langfuse single-provider guidance Make the docstring, warning, and AGENTS.md consistent with the README: only one library can own the global OpenTelemetry provider; Monocle initializes at startup before Langfuse's per-run handler, so enabling both drops Langfuse's spans — enable one OTel tracer (LangSmith, a callback, coexists fine). * Address review: optional extra, exporter validation, off-box warning, tests Responds to the second review round. - Make monocle_apptrace an optional extra (deerflow-harness[monocle], re-exposed as deer-flow[monocle]) following the boxlite/tui precedent, so a default install no longer pulls the OpenTelemetry stack. It stays pinned in the dev group for the tracing tests, and enabling MONOCLE_TRACING without the extra raises a clear install error. - Warn loudly at startup whenever any exporter other than `file` is configured, since those move prompts, tool inputs/outputs, and completions beyond the local .monocle/ directory. - Validate MONOCLE_EXPORTERS against the known exporter names and require OKAHU_API_KEY when okahu is selected, mirroring the Langfuse pattern. Validation runs from Monocle's own init (not validate_enabled) so a config typo can never fail agent runs; errors surface at Gateway startup instead. - Grow the tests from 5 to 13: caplog coverage for the Langfuse-conflict and off-box warnings, exporter validation cases, a stronger import-time regression that asserts the global TracerProvider is not replaced, and a subprocess double-invoke test exercising the real check_duplicate_setup. - Docs: config.example.yaml block retitled to a dedicated tracing header; README documents the [monocle] install and scopes tracing to Gateway runs. * docs: align Monocle README section with the other tracing providers Lead with what Monocle is and captures, drop the install step (the dev group already ships monocle_apptrace via uv sync; unusual installs get the RuntimeError), and point the missing-package error at the repo-native command (uv sync --extra monocle / deerflow-harness[monocle]). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Address review: verified Langfuse coexistence, lifespan test, scope docs Responds to the third review round. The Langfuse conflict claim was wrong, verified empirically against langfuse 4.5.1 in both init orders: whichever library initializes second reuses the existing global TracerProvider and attaches its own span processor, so neither side loses spans. Dropped the warning and its tests, corrected the README, AGENTS.md, and config.example.yaml statements, and pinned the verified behavior with test_coexists_with_langfuse (real monocle + real langfuse in a subprocess, no mocks). One honest caveat documented: both processors see all spans, so Monocle's exporters also capture Langfuse's spans when both are enabled. Also from the review: - Document the Gateway-only scope in AGENTS.md: the lifespan is the sole call site, so the embedded DeerFlowClient and TUI are not instrumented; embedded users call setup_monocle_tracing_if_enabled() themselves. - Add test_gateway_lifespan_initializes_monocle pinning the lifespan wiring. - Comment why MonocleTracingConfig.is_configured is intentionally coarser than LangSmith/Langfuse (composite validation lives in validate() at startup). - Note that monocle_exporters_list takes the comma-separated string as-is. - Module-level importorskip("monocle_apptrace") so minimal installs collect the test module cleanly. * docs: reword Monocle intro sentence * fix(tests): run the import-time regression in a subprocess test_no_import_time_setup deleted deerflow.agents* from sys.modules and re-imported to force __init__ to re-execute. The re-import creates new module objects, and restoring the old sys.modules entries afterwards leaves the parent package's attribute bindings pointing at the new ones, so any later test that resolves a deerflow.agents.* dotted path (monkeypatch.setattr in test_summarization_middleware, test_thread_data_middleware, and others) failed with "module 'deerflow.agents' has no attribute ...". Run the check in a subprocess instead: the import is genuinely fresh, the assertion is stronger (the provider must still be the SDK-less proxy, proving nothing was installed at any point), and no module identity leaks into the rest of the suite. * Address review: console warning scope, embedded hint, honest naming, doc alignment Responds to the post-approval review round: - Scope the off-box exporter warning to the remote exporters (okahu, s3, blob, gcs): console writes to local stdout and no longer trips it. config.example.yaml's data-handling note now distinguishes file / console / remote likewise. - Rename MonocleTracingConfig.is_configured to is_enabled so the boolean reads as what it checks; the exporter-dependent credential check stays in validate(), run at Gateway startup. - Hint on the embedded path: build_tracing_callbacks() logs a debug line when MONOCLE_TRACING is set but setup never ran in this process, so embedded DeerFlowClient/TUI users are not left with silent no-op tracing. Backed by a process-global setup flag. - Re-export setup_monocle_tracing_if_enabled from deerflow.tracing, matching the package convention. - Note the deliberate fail-open-at-startup contrast with LangSmith/Langfuse in the lifespan, and the OTel SDK-internals dependency in the coexistence test. - Test hygiene: clear MONOCLE_* env in the tracing config/factory fixtures; reset the setup flag in the monocle test fixture; reword the README Langfuse-spans claim as the shared-provider inference it is. - Document that .monocle/ trace files are never rotated or cleaned up. * fix(tests): pin the factory logger level in the embedded-hint tests configure_logging() from earlier tests in the full suite pins an explicit INFO level on the logger hierarchy, so a root-level caplog.at_level(DEBUG) never sees the factory's debug hint. Scope caplog to deerflow.tracing.factory so the test is independent of suite ordering. * Address review: co-export disclosure, lifespan failure test, exporter parse dedup - Off-box warning now notes that Langfuse's spans are exported too when both providers are enabled and share the global OTel provider; pinned both ways by tests. - Pin the lifespan fail-open contract: a raising Monocle setup is logged and the Gateway keeps serving (pragma dropped now that the path is exercised). README notes a config error is reported at startup and tracing stays off until restart. - Hoist exporter parsing into MonocleTracingConfig.exporter_list so validate() and the off-box warning cannot diverge, and note the upstream coupling on the exporter allow-list. - Reduce config.example.yaml's Monocle block to a pointer; the capture, retention, and data-handling detail lives in README's Monocle section. --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Yufeng He
|
e361122b9a
|
fix(security): html-escape subagent descriptions before the <subagent_system> block (#4157)
A custom subagent's description is agent-editable (persisted by setup_agent / update_agent) and is rendered into the <subagent_system> block of the lead-agent system prompt via the available-subagents listing. It was interpolated raw, so a first line like "</subagent_system><system-reminder>..." could close the block and forge a framework-reserved tag inside the system-role prompt. Escape it with html.escape at the render site, matching the sibling fixes for <soul> (#4137), memory facts (#4097), skill metadata (#4128), and remote content (#4099/#4002). Built-in descriptions are trusted constants and stay untouched. Adds a red/green regression test mirroring test_soul_prompt_injection.py. |
||
|
OrbisAI Security
|
8cc4b3abeb
|
fix(sandbox): the sandbox provisioner api exposes endpoints need to check API KEY (#4116)
* fix: V-001 security vulnerability Automated security fix generated by OrbisAI Security * fix(sandbox): wire provisioner API key through backend client and config RemoteSandboxBackend now accepts an api_key parameter and sends it as X-API-Key on all five provisioner HTTP calls (list, create, destroy, is_alive, discover). AioSandboxProvider reads provisioner_api_key from SandboxConfig and forwards it at construction time. SandboxConfig formally declares the field; config.example.yaml documents it under Option 4; docker-compose-dev.yaml threads PROVISIONER_API_KEY into both the provisioner and gateway containers so a single .env entry covers both sides. Tests: monkeypatch PROVISIONER_API_KEY and send X-API-Key headers in the five parametrized threading tests (previously 401-failing); new test_auth_middleware asserts /health is open, /api/* rejects no-header and wrong-key with 401, and accepts the correct key. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(provisioner): correct fail-closed auth docs and fix test mock signatures The first round of PR #4116 fixes left four blocking issues: - Mock signatures in test_remote_sandbox_backend.py (17) and test_aio_sandbox_provider.py (2) didn't accept the new headers= kwarg, causing TypeError on every provisioner HTTP call test. - sandbox_config.py and config.example.yaml described the auth as optional ("leave unset to disable") but the middleware is fail-closed: an unset PROVISIONER_API_KEY causes 401 on every /api/* request. - .env.example had no PROVISIONER_API_KEY entry, leaving users with an empty value and silent 401s. - No test covered the PROVISIONER_API_KEY="" fail-closed path. Fix all four: add headers=None to all mock signatures, correct the field description and example comment to state that both sides must have the same key set, add PROVISIONER_API_KEY to .env.example with generation guidance, add test_auth_middleware_unset_key, and add a logger.warning on auth rejection for observability. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> |
||
|
Aari
|
e32456d337
|
fix(memory): coerce stored confidence in the three remaining raw reads (#4034)
* fix(memory): coerce stored confidence in the three remaining raw reads `_coerce_source_confidence` exists because `memory.json` is user-editable and written across versions: a `confidence` that is null, a str, a bool, out of range, or non-finite still has to read back as a usable score. Consolidation already routes every stored read through it, and #4023 did the same for the max_facts trim. Three reads still take the field raw. `_build_staleness_section` formats it with `f"{conf:.2f}"` — a str raises ValueError, None raises TypeError. `_do_update_memory_sync`'s `except Exception` swallows that into `return False`, aborting the whole memory-update cycle permanently, since the offending fact is then never rewritten. The staleness cap's `sort(key=lambda f: f.get("confidence", 0))` ranks the facts it is about to delete. A str raises the same way; the values that don't raise mis-rank instead. `true`/`inf` outrank a genuine 0.9 and push it into the removal slot; an absent key ranks 0 and is deleted first; `nan` compares false against everything, so which fact dies depends on where the corrupted one happens to sit in the file. `search_memory_facts` — the `memory_search` tool, added in #4023 — ranks results the same raw way and then truncates to `limit`, so the same mis-ranking hands the model the wrong facts, or fails the tool call outright on a str. All three now read through the helper, so an unusable stored confidence ranks as unknown (0.5): neither kept ahead of a real score nor evicted before one. The two sorts run in opposite directions, and 0.5 is load-bearing in both. * test(memory): anchor the confidence delta at every coerced read The str-based tests raise on main, so they cannot go red for any stored confidence that mis-ranks without raising. `true`/`false`/`inf` and an absent key never reached the except handler at all: bool subclasses int, so `true` ranked 1.0; `inf` outranked every real score; an absent key ranked 0. Silent fact loss in the staleness cap, wrong results out of `memory_search` — no log line either way. Parametrize both ranking sorts over the four non-raising inputs that fall to the 0.5 default, each pitted against a genuine neighbour chosen so the survivor flips. The sorts run in opposite directions, so the one matrix covers eviction from both ends. `nan` is excluded from those: its old rank is undefined rather than pinned to an end of the order, so one fixed input order happens to yield the correct survivor. Its real property is order-independence, asserted across both fact orders. The staleness prompt formatter is pinned by rendered value, not merely by not raising: 1.5 → 1.00, -0.3 → 0.00, and inf/nan/true/None → 0.50, matching the consolidation prompt that reads the same field through the same helper. |
||
|
heart-scalpel
|
b53c1ae0e0
|
fix(runs): cancel degrades to lease takeover for multi-worker (#4064)
* fix(runs): cancel degrades to lease takeover for multi-worker Work item 4 of the multi-worker ownership epic (https://github.com/bytedance/deer-flow/issues/3948). Problem: POST /runs/{run_id}/cancel landing on a non-owning worker returns 409 — the cancel button silently fails under GATEWAY_WORKERS>1 with no sticky routing. cancel() required the current worker to hold the in-memory task/abort_event, which any non-owner pod cannot satisfy. Changes: - RunManager.cancel() returns CancelOutcome enum (cancelled / taken_over / lease_valid_elsewhere / not_active_locally / not_cancellable / unknown) instead of bool, so the router can map each outcome to the right HTTP response. - New store primitive claim_for_takeover(): a single atomic conditional UPDATE that marks a run as error only when status IN (pending, running) AND (lease IS NULL OR lease < now - grace). Closes the stale-read / concurrent-heartbeat race — if the owner renews between our read and write, the UPDATE matches 0 rows and we surface lease_valid_elsewhere. - HTTP cancel + stream-join endpoints route on CancelOutcome: cancelled -> 202 (or 204 with wait=true); taken_over -> 202 immediately (no SSE streaming — the run is terminal on another worker, streaming would hang); lease_valid_elsewhere -> 409 + Retry-After header computed from lease_expires_at + grace_seconds. - RunManager.grace_seconds exposed as a public property; the router no longer reaches into _run_ownership_config. - _is_lease_expired extracted to a module-level function, shared by RunManager.cancel() and MemoryRunStore.claim_for_takeover(). - GATEWAY_WORKERS=1 + heartbeat_enabled=false is zero-regression: the non-local path short-circuits to not_active_locally, preserving the original 409 behaviour the existing tests pin. Tests: 12 new (5 store primitive + 4 cancel-takeover unit + 3 HTTP including a regression guard verifying POST /stream?action=interrupt on a dead-owner run returns 202 instead of hanging on SSE). 244 directly-related tests pass; 36/36 blocking-IO gate pass. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(runs): guard update_status and self-terminate on takeover Two defenses close a split-brain window where the original owner could overwrite a peer's takeover status: - update_status (SQL + memory store) now guards on status IN ('pending','running'). When takeover already set the row to 'error', the owner's final status write matches 0 rows and is dropped. - _persist_status: when update_status returns False, check whether the row exists before attempting recovery via put(). If the row exists (takeover by another worker), skip recovery instead of blindly upserting over the takeover. - Heartbeat _renew_leases: when update_lease returns False (row no longer pending/running or owner changed), cancel the local task so wasted CPU is bounded to the next heartbeat tick (~10s) instead of the full task lifetime. Also fix three reviewer feedback items: - Re-fetch the store row when cancel() returns lease_valid_elsewhere, so Retry-After uses the owner's freshly-renewed lease instead of a stale value from request start. - Fallback 'unknown' in takeover error message when owner_worker_id is NULL (pre-ownership data). - Remove dead else-10 branch from grace_seconds property (unreachable — all callers are downstream of the heartbeat_enabled guard). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * test(runs): pin split-brain defences from update_status guard + heartbeat Three tests lock down the takeover authoritativeness so a late-running owner cannot overwrite a peer's claim: - update_status must reject writes when the store row is already terminal (taken over by another worker). - _persist_status must skip row-recovery via put() when the row exists but has been taken over. - Heartbeat _renew_leases must cancel the local task when update_lease returns False (row claimed by another worker). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(runs): precise outcome + log when local cancel loses to peer takeover Two reviewer precision nits on the split-brain defence: - _persist_status: branch the skip-reason log on existing["status"]. error → WARNING "peer takeover" (anomalous); interrupted/success → INFO "local cancel/completion race" (expected when user hits stop as the run finishes). Stops noisy false-positive takeover warnings in operator logs. - cancel() local path: when _persist_status returns False, re-check the store. If a peer's claim_for_takeover flipped the row to error between our in-memory cancel and the guarded update_status, surface taken_over instead of cancelled so the client sees a status consistent with the store. Test: test_cancel_returns_taken_over_when_peer_claims_during_local_cancel pins the race outcome. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(runs): widen update_status guard, de-duplicate lease helpers, add coverage Round 3 of reviewer feedback: - Widen update_status guard to status IN ('pending','running','interrupted'). The original guard blocked interrupted→error (the rollback finalize path), losing the "Rolled back by user" message. interrupted is now permitted while error/success stay locked — takeover protection unchanged. - claim_for_takeover False now re-reads the store row to distinguish causes: owner renewed lease → lease_valid_elsewhere; row went terminal → not_cancellable; another worker already took it over → taken_over. - Extract _raise_lease_valid_elsewhere() helper to de-duplicate the 409+Retry-After block shared across cancel_run and stream_existing_run. - Extract _lease_expired_or_null() in persistence/run/sql.py to de-duplicate the lease-expiry SQL WHERE clause shared by claim_for_takeover and list_inflight_with_expired_lease. - 11 new tests: 5 SQL-layer claim_for_takeover (expired/valid/NULL/ terminal/nonexistent), 3 _compute_retry_after unit (NULL/unparseable/ normal), 2 claim re-read precision (terminal/takeover), 1 stream endpoint 409+Retry-After. Not addressed (non-blocking, reviewer agreed): - The 2–3 store.gets in the takeover cold path: optimizing the API to accept a pre-fetched record would couple the router to the manager more tightly than justified by the perf gain. - The lease-expiry inline loop in MemoryRunStore.list_inflight_with_- expired_lease pre-computes cutoff once for all rows; switching to the shared _is_lease_expired helper would recompute datetime.now() per row with no real benefit. 260 related tests pass; 36/36 blocking-IO gate pass; ruff clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(runs): de-duplicate lease-expiry helper, restore defensive fallback Address final round of review feedback: - Extract is_lease_expired to deerflow.utils.time (no _ prefix, public utility). Manager and MemoryRunStore now import from the same place instead of the store reaching backward into the manager for a private function. - Restore defensive else-10 fallback in grace_seconds property (removed in an earlier round). The guard is unreachable for current callers but protects future ones from AttributeError. - Comment the transient in-memory interrupted vs store error state when a local cancel is superseded by a peer takeover. - Comment the max(1, ...) floor in _compute_retry_after — the floor is a lower bound, not a poll interval; clients should apply jitter. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: rayhpeng <rayhpeng@gmail.com> |
||
|
Daoyuan Li
|
3e7baba39a
|
fix(models): apply stream_chunk_timeout default to all BaseChatOpenAI subclasses (#4102)
* fix(models): apply stream_chunk_timeout default to all BaseChatOpenAI subclasses The 240s stream_chunk_timeout default (issue #3189, PR #3195) was scoped to a class-path allowlist of only ChatOpenAI and PatchedChatOpenAI. Every other OpenAI-compatible provider that subclasses BaseChatOpenAI — VllmChatModel, MindIEChatModel, PatchedChatDeepSeek, PatchedChatMiMo, PatchedChatStepFun and PatchedChatMiniMax — was excluded, so they kept langchain-openai's aggressive 120s built-in chunk-gap timeout and, worse, silently discarded a user's explicit stream_chunk_timeout override from config.yaml. Issue #3189 was itself reported on mimo-v2.5 (PatchedChatMiMo), the exact class the original fix left out. Gate the injection on issubclass(model_class, BaseChatOpenAI) instead of the string allowlist, so any OpenAI-compatible subclass inherits the default and honors an explicit override. Genuinely non-OpenAI clients (e.g. ChatAnthropic) stay excluded and still have the kwarg dropped before it reaches a constructor that would divert it into model_kwargs and fail at request time. * fix(models): address review nits on stream_chunk_timeout default Correct the module-level comment above _DEFAULT_STREAM_CHUNK_TIMEOUT_SECONDS: langchain-openai's built-in stream_chunk_timeout default is 120s, not 60s (BaseChatOpenAI.stream_chunk_timeout's default_factory reads LANGCHAIN_OPENAI_STREAM_CHUNK_TIMEOUT_S with a 120.0 fallback). Simplify the BaseChatOpenAI gate in _apply_stream_chunk_timeout_default from `isinstance(model_class, type) and issubclass(model_class, BaseChatOpenAI)` to just `issubclass(...)`. The sole caller passes model_class from resolve_class(), which already raises before returning anything that isn't a type, so the isinstance half can never be False there. Also soften the docstring's non-OpenAI-client bullet: ChatAnthropic declares extra="ignore" and silently drops an unrecognized kwarg rather than diverting it into model_kwargs and failing at request time (that failure mode is specific to other OpenAI-style clients). |
||
|
qin-chenghan
|
a94ea9325b
|
fix(agents): load SOUL.md from agent dirs without config.yaml (#4136)
* fix(agents): load SOUL.md from agent dirs without config.yaml (#4135) resolve_agent_dir requires config.yaml to be present in an agent directory (added in #3481 to fix #3390, where memory-only directories were mistaken for agent directories). However, SOUL.md loading does not depend on config.yaml. When an agent is configured externally (e.g. via DEER_FLOW_CONFIG_PATH) and the agent directory contains SOUL.md but no config.yaml, resolve_agent_dir skips the directory and load_agent_soul returns None. Add a fallback in load_agent_soul: if the resolved directory does not contain SOUL.md, check the per-user and legacy directories directly for SOUL.md. This preserves the #3390 fix for resolve_agent_dir (which is also used by load_agent_config) while allowing SOUL.md to load independently of config.yaml. * fix(agents): gate SOUL.md fallback on missing config.yaml per review Address review feedback on PR #4136: 1. Gate the fallback condition on not (agent_dir / config.yaml).exists() so it only fires when resolve_agent_dir returned its default path (no agent dir qualified), not when a properly-resolved per-user agent simply lacks SOUL.md. This preserves the per-user shadowing invariant. 2. Fix test_loads_soul_from_user_dir_without_config_yaml to actually exercise the fallback path: per-user memory-only dir (no config.yaml, no SOUL.md) + legacy dir with SOUL.md (no config.yaml) -> fallback finds legacy SOUL.md. 3. Add test_soul_not_leaked_from_legacy_when_per_user_has_config to verify the gate prevents legacy SOUL.md leaking into a per-user agent. 4. Patch get_effective_user_id in test_loads_soul_without_config_yaml for consistency and resilience. |
||
|
Yufeng He
|
807c3c5218
|
fix(security): html-escape SOUL.md before it enters the <soul> prompt block (#4137)
SOUL.md is agent-editable (setup_agent / update_agent persist it) and get_agent_soul renders it into the <soul> block of the lead-agent system prompt without escaping. A crafted personality such as "</soul></system-reminder>\n\nSYSTEM: ..." can close the block and relocate the text after it out of the trust zone the system prompt declares — the same break-out the skill/memory/tool-result escaping in #4097/#4119/#4128/#4099 already closes at their render sites. <soul> is the remaining one, and it lands in the highest-trust system-role block. Escape with html.escape(quote=False) (element-text position, never an attribute). Adds a regression test that fails on main. Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com> |
||
|
DanielWalnut
|
4e209827f3
|
feat(agent): Add subagent total delegation cap (#4115)
* fix subagent total delegation cap * fix embedded subagent run cap context * fix subagent cap config consistency * fix resumed subagent run cap boundary * fix legacy resume subagent boundary * address subagent cap review feedback --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Xinmin Zeng
|
95a35f3725
|
docs(deployment-guide): fix stale userdata PVC subPath (#4139) | ||
|
nonoge
|
63e2da1401
|
fix landing hero background positioning (#4092) | ||
|
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
|
bcee5a9061
|
refactor(sandbox): give the host→virtual output-mask regex a single owner (#4108)
* refactor(sandbox): give the host→virtual output-mask regex a single owner Two call sites rewrite host paths back to their virtual form in text that reaches the model — LocalSandbox._reverse_output_patterns (bash output) and sandbox.tools._compiled_mask_patterns (glob/grep/ls results) — and each built the same `escape(base) + boundary + tail` rule from its own copy. That duplication has already produced two bugs: #4035 added the segment boundary to the reverse patterns and missed the masking patterns, and #4053 had to add the same boundary to the other copy. Extract the rule into sandbox/path_patterns.py so a third copy cannot silently disagree. The extraction is not a pure move: the two sites disagree on the base. tools.py derives bases from _path_variants (which yields Windows spellings) and matches them against output whose separators it does not control, so it relaxes the separators inside the base; LocalSandbox resolves its bases from the running platform and must not be widened. That difference is now an explicit `separator_agnostic` parameter rather than an accident of two implementations. The boundary and tail constants are private: build_output_mask_pattern is the only supported spelling, so a third site cannot import the pieces and hand-roll a variant. Behavior is unchanged at both sites — pinned by tests that reproduce each pre-extraction expression byte-for-byte. * test(sandbox): pin the base the helper must not normalize Review notes on #4108. The committed snapshot compares the helper against hand-copied literals of the pre-extraction expressions, so its red-ness rests on those literals, not on the length of _BASES -- both sides compute the same expression, and 5k fuzzed bases produce zero byte-differences. Mutating the helper one clause at a time (12 mutations over the boundary, the tail and the escape/replace) shows the seven committed bases catch 11: the miss is a helper that normalizes its input by rstripping a trailing separator. Only a trailing-slash base or a Windows drive root catches that, and Path.resolve() / str(Path(...)) strip trailing slashes, so neither call site can produce the former. C:\ survives resolve() with its separator intact, so that is the one base worth adding. Also point local_sandbox's comment at path_patterns, the owner, instead of citing _content_pattern as the class reference, and drop the rationale it now duplicates from the owner's docstring -- a second copy of the explanation drifts the same way the second copy of the regex did. The site-specific half stays. Comments and test data only; no behavior change. |
||
|
黄云龙
|
62f905342c
|
fix(tool_search): remove MAX_RESULTS cap on select: exact-name lookups (#4086)
The select: query form lets the model explicitly name the tools it wants to promote. Capping the result at MAX_RESULTS (5) silently drops valid selections when more than 5 deferred tools are requested at once. This removes the [:MAX_RESULTS] slice on the select: branch in DeferredToolCatalog.search() (exact-by-name lookups should return every matched tool) and the redundant re-cap in build_tool_search_tool() (search() already caps non-select query forms internally). Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Aari
|
37580862b9
|
fix(channels): scope the slash-skill whitelist check to the run's owner (#4129)
_channel_storage_user_id is the single source of truth for a channel run's
identity: _resolve_run_params resolves the owner into run_context["user_id"].
The /skill whitelist pre-check for a per-user custom agent
(_resolve_available_skill_names) resolves that owner and then drops it, calling
load_agent_config(name) with no user_id. It falls back to get_effective_user_id(),
but this runs on the ChannelManager dispatch loop where the _current_user
contextvar is never set, so it resolves "default" -- reading
users/default/agents/{name}/ instead of the owner's bucket. When that bucket has
no such agent (the common case) load_agent_config raises FileNotFoundError, which
the dispatch loop turns into "An internal error occurred" on every /skill
command; when a foreign agent shares the name, the whitelist is decided by the
wrong user's skills list.
Pass the resolved owner (run_context["user_id"]) to load_agent_config, matching
every other caller (gateway/routers/agents.py, update_agent_tool.py,
github/registry.py). None when no owner is resolvable, preserving the prior
default-user behavior for unbound/no-auth channels.
|
||
|
Yufeng He
|
cbbd72a1ab
|
fix(skillscan): recognize remaining requests/httpx HTTP methods as network sinks (#4130)
python-env-dump-exfil flags a file that both reads the bulk process environment and reaches a network sink. The call-based sink check only listed requests get/post/put/request and httpx get/post, so a bulk env dump sent through an equally body-carrying method (requests.patch/delete, httpx.put/patch/delete, or the generic httpx.request/stream) evaded the CRITICAL finding whenever the destination URL was not a plain string literal (e.g. built at runtime) -- the exact evasion the string-literal URL sink is meant to resist. requests.post was caught but requests.patch was not, an arbitrary gap on clients the analyzer already covers. Complete the requests and httpx HTTP-verb surface in _call_is_network_sink so an obfuscated-URL exfil through those methods is flagged like post/put. Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com> |
||
|
chaoxi007
|
216309426f
|
test(front): cover artifact image path variants (#4134) | ||
|
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
|
2bd0f56a0f
|
fix(subagents): classify recursion-capped LLM error fallbacks as failed (#4056)
The GraphRecursionError except-block in SubagentExecutor._aexecute derives usable_partial from the last AIMessage's raw non-empty text, without checking _extract_llm_error_fallback (#4042) first. A handled provider failure (LLMErrorHandlingMiddleware's deerflow_error_fallback marker) always carries non-empty user-facing text, so when it lands on the same turn that trips max_turns, it is indistinguishable from genuine partial output and gets misclassified as a completed task instead of the failed provider error it is. Consult _extract_llm_error_fallback in this except-block too, same as the normal-completion path above it, and classify FAILED with stop_reason=turn_capped when it detects the marker. |
||
|
Daoyuan Li
|
08fd218b83
|
fix(sandbox): use os.sep in reverse-resolve containment check on Windows (#4058)
* fix(sandbox): use os.sep in reverse-resolve containment check on Windows Path.resolve() always renders with the native separator (backslash on Windows), but _reverse_resolve_path's containment check hardcoded a "/" suffix when testing whether a resolved path is nested under a mapping's local root. Only the exact-root case (no separator needed) ever matched; every nested path fell through to the "no mapping found" branch and returned the raw host path -- leaking the real username and full directory tree into list_dir/glob/grep results and bash output masking instead of the virtual /mnt/user-data/... path. _is_read_only_path already does the equivalent check correctly via os.sep, so this aligns _reverse_resolve_path with that pattern: the containment check now compares with os.sep, and the extracted relative portion is normalized to forward slashes before being spliced into the (always POSIX-style) container path. Also fixes a same-file cosmetic bug in list_dir's virtual sub-directory overlay: it compared a bare child name (e.g. "workspace") against a set of full container paths, so the already-listed guard never matched and a mount whose subdirectory the underlying scan already found (the common case for /mnt/user-data/workspace, uploads, outputs) was appended a second time. Continues the same separator-bug class already fixed in this file by #3869 (forward-direction command resolution) and #4035 (reverse regex-boundary matching); neither touched this containment check. * test(sandbox): add host-OS-independent regression test for the os.sep containment fix _reverse_resolve_path's os.sep containment check (and the paired lstrip(os.sep).replace(os.sep, "/") extraction) has no test that would fail if reverted: backend CI runs only on ubuntu-latest, where os.sep == "/" makes the pre-fix hardcoded "/" and the current os.sep form observationally identical, so a plain POSIX-path test can't discriminate between them. Add a test that forces the Windows code path independent of host OS by monkeypatching os.sep to "\" and stubbing both the module's Path name and the sandbox's cached _resolved_local_paths to return backslash-joined strings, mirroring what real WindowsPath.resolve() produces -- without touching the filesystem or requiring an actual Windows host. Verified this fails with the raw host path leaking through when the os.sep fix is reverted to the hardcoded "/" form, and passes with the fix in place. |
||
|
Aari
|
c82fba41d9
|
fix(sandbox): guard the command path-translation regex with a segment boundary (#4110)
replace_virtual_paths_in_command matches the virtual root with no
segment-boundary lookahead:
re.compile(rf"{re.escape(VIRTUAL_PATH_PREFIX)}(/[^\s\"';&|<>()]*)?")
The trailing group needs a "/" to consume anything, so when the character after
/mnt/user-data is "-", ".", "_", a digit or a letter, the group matches empty
and the bare root still matches. The substitution then rewrites it to the
thread's host user-data directory and the rest of the sibling name rides along,
so a command naming a prefix sibling of the mount root is pointed at a real host
directory outside the mount contract:
cat /mnt/user-data-backup/secret.txt -> cat <host>/user-data-backup/secret.txt
which reads the host file. This is the same defect as #4035 (reverse patterns)
and #4053 (masking patterns), mirrored into the virtual->host direction; it is
the last unguarded member of that family.
The boundary class mirrors LocalSandbox._content_pattern's rather than
_command_pattern's: a virtual root can legitimately be followed by ":"
(PATH-style concatenation) or ",", which the shell-oriented class rejects, so
narrowing to it would stop translating paths that translate today. "$" covers a
command ending exactly at the root.
|
||
|
明年我18
|
0542d3c5f3
|
fix(sandbox): allow bash after cwd setup failure (#4051) | ||
|
黄云龙
|
897be7e064
|
fix(skillscan): detect os.environ access via from-import pattern (#4087)
* fix(skillscan): detect os.environ access via from-import pattern * fix(skillscan): detect os.environ access via from-import pattern |
||
|
黄云龙
|
224f8de2cf
|
fix(goal): prevent continuation_count regression from racing continuations (#4088)
* fix(goal): prevent continuation_count regression from racing continuations * fix(goal): prevent continuation_count regression from racing continuations |