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>
* 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.
* 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
_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.
* 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.
* 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>
* 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>
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.
* 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>
* 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.
* 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>
* 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).
* 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.
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>
* 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>
* 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.
* 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.
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>
_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.
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>
* 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.
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.
* 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.
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.
* fix(goal): prevent continuation_count regression from racing continuations
* fix(goal): prevent continuation_count regression from racing continuations
* fix(memory): coerce null confidence when ranking stale facts
_build_staleness_section and the per-cycle removal cap in _apply_updates
used fact.get("confidence", 0.0/0), which only defaults when the key is
absent. A fact whose confidence is explicitly null (or otherwise malformed)
returned None, breaking the numeric sort/format. Use
_coerce_source_confidence, which normalizes null/malformed values and clamps
to [0, 1], so null-confidence facts no longer block staleness handling.
* ci: retrigger cancelled CI workflow
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
The parse gate required all of {user, history, newFacts, factsToRemove} to
be present before accepting the model's JSON. A well-formed update that
simply has no facts to remove (the common case) omits the empty
factsToRemove key and was silently rejected. Drop factsToRemove from the
required set so those updates parse and apply.
LoopDetectionMiddleware._get_run_id used a truthiness check that
collapsed a present-but-None run_id to the same "default" key as a
totally absent one. SubagentExecutor sets context["run_id"] =
self.run_id unconditionally, so run_id is genuinely None for an
embedded/TUI-dispatched subagent, and later reads the stop reason back
with that same raw attribute via consume_stop_reason(self.run_id). The
write (keyed "default") and the read (keyed None) disagreed, so a
genuine loop-detection hard-stop's loop_capped reason was silently
dropped instead of reaching the lead.
Align _get_run_id with TokenBudgetMiddleware's key-presence-based
version, which does not have this bug: return the context value as-is
when the key is present (None included), and fall back to a
per-runtime-unique key only when the key is absent.
str_replace returned "OK" whenever the target file was empty, silently
reporting success even when the model asked to replace a non-empty string
that could not possibly be present. Only short-circuit to "OK" when old_str
is also empty; otherwise return the standard not-found error.
search_memory_facts sorted matches by fact.get("confidence", 0), which
returns None for a fact whose confidence key is explicitly null, crashing
the sort comparison. Use _coerce_source_confidence so null/malformed
confidence values are normalized and clamped before ranking.
* feat(frontend): show real project version on About page
The About page heading was hardcoded "About DeerFlow 2.0" while the real
version is 2.1.0, kept in lockstep across backend/pyproject.toml,
frontend/package.json, and Chart.yaml by scripts/verify_versions.sh.
With the nightly workflow shipping unreleased main daily, a hardcoded
version drifts further from reality every day.
Wire the heading to the real version, nightly-aware:
- src/version.ts reads NEXT_PUBLIC_APP_VERSION (nightly override) with a
package.json fallback for release/local builds.
- about-content.ts interpolates APP_VERSION into the heading; the
"DeerFlow 1.0 and 2.0" milestone copy in acknowledgments is left as-is.
- Dockerfile adds ARG APP_VERSION / ENV NEXT_PUBLIC_APP_VERSION in the
builder stage so nightly CI can stamp the version.
- nightly.yaml computes the nightly string once in prepare
(<base>-nightly.<date>-<sha>, mirroring the chart scheme) and feeds it
to both the frontend image build-arg and the chart publish (refactored
to consume the shared output, so the displayed version and chart
version can't drift). Release builds (container.yaml) are untouched
and fall back to package.json.
Tests cover the version branches and the heading interpolation;
RELEASING.md notes the About page as a derived version consumer.
* test(frontend): assert positive About-page version value
Address review feedback on #4126: the unset-env assertion was negative-only
(checked the stale "2.0" literal was gone), so it would still pass if
APP_VERSION interpolated to "" or undefined. Mirror version.test.ts by
asserting the heading carries the real resolved APP_VERSION.
* ci(nightly): fail loudly on empty base version
Address review feedback on #4126: the prepare step's BASE (parsed from
Chart.yaml) had no empty-guard. A malformed/missing version would yield
`-nightly.<date>-<sha>` (invalid semver) that now flows into both the
chart publish and the frontend About-page version. Add `set -euo pipefail`
plus a `test -n "$BASE"` guard mirroring publish-chart. `pipefail` matters
- without it the pipeline's exit code is awk's, so `set -e` alone wouldn't
catch a grep miss.
When _process_queue found another worker already processing, it called
_schedule_timer(0), spawning a fresh Timer thread immediately and looping
tightly (spawn -> busy -> reschedule -> spawn) until the active worker
finished. Replace this with a _reprocess_pending flag: a concurrent caller
sets the flag and returns, and the active worker reschedules exactly once
in its finally block when work remains. Reset the flag in clear().
github.bot_login and trigger.mention_login were read raw in the
require_mention precedence chain, so a whitespace-only value (e.g. " ")
never fell through to the working fallback the chain documents -
Python truthiness lets " " win an `or` chain over agent.name or the
operator default. The third link (channels.github.default_mention_login)
was already correctly normalized and pinned by
test_operator_default_blank_string_treated_as_none; this closes the gap
for the other two by normalizing GitHubTriggerConfig.mention_login and
GitHubAgentConfig.bot_login once, at the config layer, via a
field_validator mirroring the pattern already used elsewhere in the
config package.
* fix(run-events): serialize seq assignment with a per-thread asyncio lock
put() and put_batch() read max(seq) and then INSERT seq+1 in separate awaits.
Two coroutines writing the same thread in one process could interleave
between the read and the insert and assign the same seq, colliding on
SQLite where the DB-level FOR UPDATE lock is weaker than Postgres. Add a
per-thread asyncio.Lock (_write_locks / _get_write_lock) around the
read-assign-insert critical section in both methods.
* address review: evict orphaned per-thread write-lock in delete_by_thread
_write_locks accumulated one asyncio.Lock per thread ever seen and never
released, leaking in the long-lived DbRunEventStore singleton. Evict the
entry after delete_by_thread when no writer holds it (lock recreated
lazily on the next write). Per @willem-bd review on #4077.
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: Claude <noreply@anthropic.com>
The dual-mode _merge_stream_text used short-circuits chunk==existing
and existing.endswith(chunk) that silently dropped legitimate delta
chunks in messages-tuple mode:
- CJK reduplication: '谢谢' tokenized as ['谢','谢'] -> buffer stays '谢'
- Repeated tokens: 'gogo' as ['go','go'] -> buffer stays 'go'
- Suffix-matching tails: 'hello' as ['hel','l','o'] -> drops 'l'
Delta payloads must always append; only strictly-longer cumulative
snapshots should replace. The fix gates the cumulative-replace branch
with len(chunk) > len(existing) and removes the dropped-shape guards.
Both copies are fixed:
- app/channels/manager.py (IM streaming to Feishu/Telegram/etc.)
- deerflow/tui/view_state.py (TUI client rendering)
The TUI reduce() caller now handles exact re-sends (values snapshot
re-emitting history) before the merge, and the len>1 guard prevents
single-char CJK deltas from being mistaken for re-sends.
Co-authored-by: Claude <noreply@anthropic.com>
get_initial_oauth_headers() now wraps each server's token fetch in
try/except so one broken OAuth server does not abort the entire
multi-server MCP tool load (which returned [] from the outer except,
dropping every server's tools).
_fetch_token() now captures a rotated refresh_token from the token
response and updates the in-memory McpOAuthConfig so subsequent
refreshes use the latest value instead of the stale original, which
fails with invalid_grant on providers that rotate refresh tokens
(Auth0, Okta, Google, etc.).
Adds regression tests:
- One failing OAuth server still yields the healthy server's headers
- Rotated refresh_token is posted on the next refresh attempt
Co-authored-by: Claude <noreply@anthropic.com>
_SubagentEventBuffer.flush() cleared self._pending before the put_batch and
discarded the batch when persistence raised, so a transient store error
silently lost subagent step events. On failure, prepend the failed batch
back onto self._pending (ahead of events queued since) so a later flush can
retry it.
* fix(sandbox): stop glob/grep/ls from surfacing disabled skills' files
The disabled-skill gate checks the path a tool is given, but ls, glob and
grep all descend from it and return other paths, so a root above a disabled
skill still serves its files. glob and grep never called the gate at all;
ls called it only on its own argument and still leaked from a category root.
Add the entry gate to glob/grep, and filter what all three return through
the existing fail-closed _is_disabled_skill_path. The verdict is memoized per
skill because ExtensionsConfig.from_file() is uncached, so a per-match check
would turn a 100-match grep into 100 config reads.
* fix(sandbox): normalize trailing slashes in the disabled-skill path check
Review follow-ups on the disabled-skill gate:
- _extract_skill_name_from_skills_path returned "" instead of None for a
category directory carrying a trailing slash. LocalSandbox.list_dir appends
"/" to directories, so `ls /mnt/skills` yields "/mnt/skills/public/", giving
parts ["public", ""]. The empty name skipped the `skill_name is None`
short-circuit and fell through to a config read, landing on the right outcome
only because unknown skills default to enabled. Drop empty segments so a
trailing-slash category root takes the existing category-root branch.
- ls_tool resolved the runtime user id twice per call; hoist it into a local,
matching glob_tool/grep_tool.
- Cover the CUSTOM path: custom/legacy skills resolve their enabled state
through the per-user _skill_states.json, a different store from the public
skills' extensions_config.json, and no automated test exercised it.
The remote-content allowlist in ToolResultSanitizationMiddleware
(`_REMOTE_CONTENT_TOOL_NAMES`) covered web_fetch / web_search /
image_search but not web_capture, which was added later. The Browserless
web_capture tool embeds the target site's `X-Response-Status` reason
phrase — free-form text controlled by whatever server is being captured
(RFC 7230 §3.1.2) — into its result message via `_target_status_warning`.
A malicious page could therefore forge a `<system-reminder>` block (or a
`--- END USER INPUT ---` boundary marker) through web_capture that would
be escaped for web_fetch, letting attacker-influenced remote content reach
the model as authoritative framework context.
Add "web_capture" to the allowlist so its result is structurally
neutralized for parity with the other remote-content tools. This extends
the same defense introduced in #4002 to the one built-in remote-content
tool it did not yet cover.
Add regression tests that build the web_capture result the way
community/browserless/tools.py does (real `_target_status_warning` +
`BrowserlessScreenshotResult`) and assert the forged tags/boundary markers
are escaped, while a benign status warning is preserved unchanged.
* fix(security): html-escape memory facts rendered into the injection prompt
The lead-agent system prompt declares the <memory> block user-managed and
everything else framework-internal, but the injection renderer _format_fact_line
formats a fact's content, category and correction sourceError raw. Memory is
user-editable via /api/memory, so a fact whose content is
'</memory></system-reminder>...' closes the block and relocates the text after
it out of the user-managed trust zone.
Escape those three fields at render time, mirroring the MEMORY_UPDATE_PROMPT
escaping added for the update-prompt side in #4028/#4060. The fact dict is not
mutated, so stored memory keeps the raw value and the apply path is unaffected.
* fix(memory): stop entity-encoding quotes in injected fact text
The three html.escape() calls in _format_fact_line used the default
quote=True, which also converts " to " and ' to '. These fields
are rendered as element text inside the <memory> block, never inside an
attribute value, so escaping quotes buys no defense here: only <, >, and &
can break out of the surrounding tags, and those are escaped either way.
Ordinary facts ("User's preference", 'Said "use Python"') reached the model
as User's preference / Said "use Python" -- content the
lead-agent prompt declares as user-managed data the model should discuss
freely. Pass quote=False and extend the benign-content test, which used a
string with no quotes and so never exercised this path.
Note the escape in updater.py's consolidation_candidates block renders into
an XML attribute value and correctly keeps quote=True.
