1066 Commits

Author	SHA1	Message	Date
Daoyuan Li	40c4ec32f4	fix(blocking-io): trace self/cls attribute chains and local aliases in the call graph (#4200) ... * fix(blocking-io): trace self/cls attribute chains and local aliases in the call graph _record_call_ref only recorded a call-graph edge for bare-name calls and literal self./cls. single-hop attribute calls (self.flush()). Any other receiver shape fell through the "." not in call_name fallback and was silently dropped from the graph -- including a deeper self./cls. attribute chain (self.store.flush()), a local variable holding a self./cls. attribute (store = self.store; store.flush()), or a parameter used directly as a receiver. A real blocking call reachable from async code only through one of those shapes never surfaced as a finding, the opposite (and more dangerous) failure mode from the duplicate-helper-name over-report this detector already documents. Trace those shapes back to a self./cls. attribute or a parameter, within the same function only, and resolve them through the same bare-method-name fallback already used for receivers that cannot be resolved to a name at all -- no new false-positive risk beyond what that existing fallback already accepts. * fix(blocking-io): narrow alias tracking to fix three scope-creep bugs The alias/receiver tracking this detector added reused dotted_name(), which intentionally unwraps ast.Call/ast.Subscript for blocking-call pattern matching elsewhere in this module. Reusing it for alias extraction let a Call or Subscript result inherit its base's alias-worthiness, so factory().flush(), client = factory(); client.flush(), and client = clients[0]; client.flush() were all incorrectly treated as calls on a traced receiver. Add _simple_receiver_name(), a restricted Name/Attribute-only extractor, and use it wherever a receiver/alias is extracted instead of dotted_name(). Alias state also only ever grew: _record_local_receiver_alias_targets never removed a name once traced, so a later reassignment to a non-traceable value (client = NonBlockingClient()) left the name aliased forever, still exposing unrelated same-named methods. Reassigning now resolves traceability from scratch and kills the name when the new value isn't traceable. Separately, if/else branches had no isolation: with no visit_If override, body and orelse shared one mutable alias set, so an alias added in one leaked into the other and the result depended on which branch was textually first. Add a visit_If override that snapshots aliases before the branch, resets between body and orelse, and unions their exit states afterwards -- a conservative, order-independent may-alias join. Scoped to ast.If only; ast.Try/ast.Match keep the previous unisolated traversal (different, more complex control-flow semantics, out of scope here). Finally, _visit_function pushed the new function's context before visiting decorator_list/args/returns, but those expressions run at definition time in the enclosing scope, not the function body. A default value referencing an outer name that happens to match one of the function's own parameter names (receiver = Store(); async def route(receiver=receiver.flush())) was misattributed to route itself. Visit decorators, parameter defaults/annotations, the return annotation, and PEP 695 type-parameter bounds before pushing the new function's context so they resolve against the enclosing scope. Real-scanner output against the actual backend tree is unchanged (41/41 findings, byte-identical JSON) -- these were latent false-positive/negative risks in shapes the current codebase doesn't happen to contain, not active miscounts. * fix(blocking-io): fix three more alias-tracking and definition-time bugs _record_local_receiver_alias_targets ran before the assignment's own value was visited, so an assignment's RHS was analyzed against the alias state *after* the target had already been updated/killed for this same statement. Python evaluates the RHS before binding the target: with `client = self.store` followed by `client = client.flush()`, the second statement's target update killed `client`'s alias before its own RHS (`client.flush()`) was visited, so that call silently disappeared from the graph. visit_Assign and visit_AnnAssign now visit the RHS first and only update the target's alias afterward, matching Python's own evaluate-then-bind order. _simple_receiver_name still returned the trailing attribute name whenever its recursive parent lookup came back unsupported (a Call or Subscript), instead of refusing the whole chain -- so `factory().client` and `clients[0].client` both collapsed to plain "client", which, when "client" was also a traced parameter or local alias, incorrectly linked `factory().client.flush()` to an unrelated same-file `Store.flush`. Return None instead of falling back to `node.attr`, so an unsupported node anywhere in the chain makes the whole receiver unresolved rather than a truncated suffix of it. Finally, _visit_function's enclosing-scope walk of decorators, defaults, annotations, and type_params recursed into every subexpression uniformly, including ones that don't actually execute at definition time: a lambda's body, a bare generator expression's element/later-for clauses, annotations postponed by `from __future__ import annotations`, and PEP 695 type-parameter bounds (always evaluated lazily, in their own hidden function, only if something like T.__bound__ is ever accessed). Add visit_Lambda/visit_GeneratorExp overrides that stop at exactly the eager subset (a lambda's own parameter defaults; a generator's outermost iterable), skip parameter/ return annotations entirely once a `postponed_annotations` flag is set by the future import, and drop the type_params walk instead of moving it to the enclosing scope. Real-scanner output against the actual backend tree is unchanged (41/41 findings, byte-identical JSON) -- these were latent risks in shapes the current codebase doesn't happen to contain, not active miscounts. * fix(blocking-io): preserve eager traversal for immediately invoked lambdas and consumed generators visit_Lambda/visit_GeneratorExp (added last round to stop treating merely created lambda/generator objects as executing at definition time) were unconditional, so they also suppressed bodies that genuinely execute right away: an immediately invoked lambda ((lambda: ...)()) and a generator expression passed directly to an eager-consuming builtin (list/set/tuple/ frozenset/dict/sorted). visit_Call now marks a Lambda used as its own func, or a GeneratorExp passed as the sole argument to one of those builtins, by node identity before generic_visit runs. visit_Lambda/visit_GeneratorExp check that marker and, on a match, visit the node fully instead of applying the lazy walk. A lambda/generator that is merely created, stored, passed as a callback, or invoked later through a variable is unaffected and stays lazy. * fix(blocking-io): scope lambda/generator laziness to definition-time expressions only visit_Lambda/visit_GeneratorExp were unconditional overrides, so they suppressed lambda bodies and generator elements everywhere the visitor reached one, not only inside another function's definition-time expressions (decorators, parameter defaults/annotations, return annotation) where that suppression is actually needed. In ordinary function-body code this caused real false negatives: a lambda stored in a local and called through that name (callback = lambda: os.listdir("."); callback()), a generator reduced by sum/any/all/min/max, a bare lambda/generator that is merely created, and a generator wrapped in another lazy iterator like map(...) all went unscanned, even though none of them are definition-time expressions at all. The previous fix for this (an id()-keyed marker set covering exactly two eager shapes -- an immediately invoked lambda, and a generator passed directly to a fixed list of eager-consuming builtins) narrowed the suppression back down, but only for those two shapes, and the underlying eager-consumer builtin set itself excluded true reducers (sum/any/all/ min/max) that consume their generator argument just as eagerly as list/ set/etc. Both are instances of the same problem: enumerating every shape in which a lambda or generator happens to be invoked/consumed piecemeal inside an AST visitor, which is unbounded in the general case. Replace both mechanisms with a single boolean, _in_definition_time_expression, set only while _visit_function walks another function's own decorators/defaults/annotations/return annotation. visit_Lambda/visit_GeneratorExp apply their lazy (defaults-only/outermost-iterable-only) walk only while it is set; everywhere else they fall through to a full generic_visit, scanning lambda bodies and generator elements unconditionally -- the same conservative, over-report-rather-than-infer stance this file already takes for reachability elsewhere. This removes EAGER_ITERABLE_CONSUMER_NAMES and the two identity-marker sets entirely rather than growing them further. The one shape this newly gives up on -- an immediately invoked lambda or eagerly consumed generator used as another function's decorator/default/annotation value -- is now an explicit, narrow, documented limitation (see backend/AGENTS.md): definition-time expressions never scan a nested lambda body or generator element, full stop, regardless of whether it happens to be invoked right there. Targeted suite (test_detect_blocking_io_static.py + test_scan_changed_blocking_io.py + test_detector_repo_root.py + blocking_io/test_gate_smoke.py): 65/66 pass, the one failure a pre-existing Windows path-separator comparison unrelated to this file. Full backend suite: identical 64 pre-existing failures on both the pre-fix and post-fix commit, confirmed by diffing the two failure lists directly -- zero regressions. Real scanner against the actual backend tree: 41/41, byte-identical JSON before and after -- these were latent risks in shapes the current codebase doesn't happen to contain, not active miscounts.	2026-07-22 13:55:40 +08:00
Ryker_Feng	20debf9cc7	feat(agents): per-agent model and generation settings (#4347) ... * feat(agents): per-agent model and generation settings Let each custom agent choose its own model and sampling settings (temperature, max_tokens) plus thinking / reasoning_effort defaults, so agents sharing a model profile are no longer stuck with one shared temperature and output length (#4336). AgentConfig gains optional model_settings / thinking_enabled / reasoning_effort (None = inherit). create_chat_model applies per-caller model_overrides on top of the profile before the thinking/Codex transforms; the lead agent resolves each knob with precedence request > agent config > profile/default. The /api/agents create/update routes persist the fields and reject an unknown model. The default lead agent path is unchanged (no agent config -> overrides None). The agent chat composer also stops force-overriding an agent's configured default model with models[0]. * fix(agents): tri-state thinking control and default-model capability gating The model-settings dialog seeded the thinking switch to false, so opening it to tweak temperature and saving silently disabled thinking (the runtime default is on) with no way back to inherit. It also hid the thinking / reasoning controls whenever the agent inherited the global default model, since `__default__` never resolved through `models.find`. Give thinking an explicit Inherit / On / Off tri-state so an untouched save is a no-op, and resolve `__default__` to the effective default (models[0]) for the capability check. Logic lives in the tested helpers module.	2026-07-22 13:44:55 +08:00
Andrew Chen	ae510cb2e8	fix(sandbox): make an empty old_str a no-op in str_replace on any file (#4256) ... str_replace guards the replacement with `if old_str not in content`, which cannot reject an empty old_str -- `"" in content` is always true. So an empty old_str reached `str.replace("", new_str)`, which inserts new_str at every character boundary, and the tool rewrote the file while still returning "OK": old_str='', new_str='# H\n' -> OK, file silently prepended old_str='', new_str='X', replace_all -> OK, 'XdXeXfX XmXaXiXnX(X)X:X\nX...' The empty-file branch above it already handles this case (`if not content: if not old_str: return "OK"`), and the existing test states the intent directly: "An empty old_str is a no-op edit and remains a benign OK". That contract just never held once the file had content. The tool is registered by default (config.example.yaml) and its schema declares old_str as a plain string with no minLength, so a model can emit "" legitimately; read-before-write only compares a hash and lets it past. Check old_str first so the no-op holds whatever the file contains. The empty-file case folds into the same not-found branch, which keeps its message and behaviour. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-22 09:20:54 +08:00
Daoyuan Li	495e90832c	fix(sandbox): scope e2b grep() glob filter to its directory prefix (#4168) ... E2BSandbox.grep()'s glob handling reduced a directory-scoped pattern like "src/*.js" down to just "*.js" before passing it to `grep --include=`, dropping the directory-scoping prefix entirely. GNU grep's `--include` matches by basename only, at any depth, so the search silently broadened to every matching-extension file in the sandbox tree instead of just the directory the caller asked for. Keep the basename portion as a coarse `--include=` pre-filter (a superset of the true match set) and post-filter grep's raw hits through path_matches(), the same helper glob() already uses to enforce directory scoping correctly, so grep and glob agree on what a directory-scoped pattern means.	2026-07-22 09:11:45 +08:00
March7	ce4a6d4c3d	fix(backend): remove transient image context after model calls (#4267) ... * fix(backend): discard transient image context * fix(backend): protect client image context ids * docs(backend): clarify image checkpoint lifecycle	2026-07-22 08:41:50 +08:00
Vanzeren	42baed8c8c	feat(checkpoint): dual-mode checkpoint storage with LangGraph DeltaChannel (#4292) ... * feat(checkpoint): dual-mode checkpoint storage with LangGraph DeltaChannel Add a restart-required database.checkpoint_channel_mode ("full" default, "delta") that stores the messages channel via LangGraph 1.2 DeltaChannel, cutting checkpoint storage from O(n^2) to O(n) for append-only history. Existing full checkpoints seed delta state transparently; no data migration. - config: mode schema + freeze-on-first-use with CheckpointModeReconfigurationError; mode marker persisted in checkpoint metadata; unsafe delta->full downgrade rejected fail-closed with CheckpointModeMismatchError (run-level error, failed state read) - state: delta message state schema; CheckpointStateAccessor centralizes materialized reads for all consumers (threads API, branches, regeneration, compaction, state updates, memory, goal workers) - runtime: raw writers (run durations, interrupted title, thread goal) parent their checkpoints to the checkpoint they derive from, preserving delta ancestry; rollback forks the pre-run lineage through a state mutation graph with Overwrite restores; InMemorySaver delta-history override delegates to the base walk (fixes dropped first write after migration, also present upstream) - tests: conformance suite over {memory, sqlite, postgres} covering migration replay, stable message IDs, storage shape and writer preservation; conftest fixture isolates the frozen mode between tests; stale config fakes refreshed - ci: backend unit tests gain a postgres service * fix(checkpoint): close materialization gaps in goal flow, guard public factory - Route goal-continuation message reads through CheckpointStateAccessor: raw channel_values reads see the delta sentinel in delta mode, which disabled goal continuation (stand_down=no_durable_end_of_turn) after durable assistant turns. Raw tuples remain for tuple-only metadata (checkpoint id, pending_writes). - Reject checkpoint_channel_mode='delta' + checkpointer in create_deerflow_agent at construction: factory-built persisted graphs bypass mode-marker injection and the fail-closed gate, reproducing silent mixed-mode state loss. Delta without persistence stays allowed. - Import the postgres saver lazily (pytest.importorskip in the fixture) so the documented default install collects the suite; add a CI job running pytest --collect-only on uv sync --group dev without extras. - Fix test_checkpointer fallback test to patch get_app_config at its use site (provider module), making it deterministic when a local config.yaml selects a persistent backend. * fix(gateway): preserve extension-owned channels in state mutations, bump config version - build_state_mutation_graph / build_checkpoint_state_mutation_accessor accept an explicit state_schema; branch and POST /state now compile the mutation graph from the thread's effective schema (graph_state_schema on the assistant graph). The base-ThreadState fallback silently discarded channels contributed by custom AgentMiddleware.state_schema on branch (data loss) and returned a false-success 200 on POST /state. - POST /state validates values keys against the mutation graph's channels and rejects unknown fields with 422 instead of ignoring them; reducer detection covers extension channels (BinaryOperatorAggregate or DeltaChannel) so Overwrite replace semantics work for middleware reducers in both modes. - Endpoint regression: custom AgentMiddleware.state_schema value survives branch, updates through POST /state, and an unknown field receives 422. - config_version 26 -> 27 for the new database.checkpoint_channel_mode (example, Helm chart values + README, support-bundle fixture), so existing installs get the outdated-config warning and make config-upgrade merges the field; covered by a test driving the real example file and the real config-upgrade script. * fix(gateway): resolve assistant schema via one boundary, copy branch reducer values with Overwrite GET /threads/{id}/state now resolves the thread's assistant_id through a single reusable boundary (thread metadata -> assistant_id -> effective graph), so channels contributed by a custom AgentMiddleware.state_schema are materialized instead of dropped by the default lead schema. POST /state uses the same boundary instead of resolving the schema ad hoc. Branch writes wrap every copied reducer channel in Overwrite (derived from the effective mutation graph: BinaryOperatorAggregate + DeltaChannel), not just messages, so already-aggregated values are never re-merged. Regression tests use a real AgentMiddleware.state_schema with a non-identity reducer in both full and delta modes: GET /state returns the extension value, POST /state replaces it, branch preserves it byte-for-byte; the unknown-field 422 is a separate assertion. * refactor(checkpoint): collapse read-path round-trips and ship dual-mode parity tests Address review round 4 on PR #4292: - Push ahistory/history limit through Pregel into checkpointer.alist (SQL LIMIT) instead of materializing all rows and breaking in Python - Fold the read-side mode-compat gate onto the returned snapshot's metadata; only writes keep the pre-write tuple fetch (fail-closed) - Cache factory-built accessor graphs per (assistant_id, mode) with factory-identity revalidation; state reads no longer build a lead agent per request - get_thread: one snapshot fetch + one raw pending_writes fetch on the resolved checkpoint (post-checkpoint __error__ writes never surface in snapshot.tasks; verified empirically) - DeerFlowClient.get_thread: single checkpointer.list walk collects pending_writes per checkpoint instead of N get_tuple calls - InMemorySaver delta-history patch: stand-down when the upstream override disappears, try/except guard, validated-version warning, guard tests - make_lead_agent mode precedence: first freeze is owned by app_config (client-supplied configurable key ignored); once frozen, injected key/app_config must match or fail closed - Rollback: lock in non-message channel restoration via fork inheritance with a dedicated reducer-channel test - Add tests/test_threads_checkpoint_mode.py and tests/test_gateway_checkpoint_mode.py referenced by AGENTS.md and the PR validation section: lifecycle parity (memory + sqlite), per-step blob-count storage guard, gateway endpoint parity Counted-saver tests pin checkpoint round-trips for aget/ahistory so these regressions cannot silently return. * fix(checkpoint): precise mode-mismatch HTTP mapping, gate E2E, and accessor resilience - threads router: map CheckpointModeMismatchError to 409 (with cause and thread id) and CheckpointModeReconfigurationError to 503 across all state endpoints instead of swallowing both into a generic 500 - gate coverage: seed a real delta checkpoint into AsyncSqliteSaver and assert aget/aupdate/ahistory fail closed; assert 409 at the HTTP boundary through the real route stack - rollback: compile the restore mutation graph with the thread's effective state schema per the build_state_mutation_graph contract - inheritance contract locks: rollback and manual compaction preserve middleware-contributed channels via checkpoint fork cloning - services: revalidate the accessor-graph cache against app_config identity so config.yaml hot-reloads never serve a stale compiled graph - services: degrade full-mode state reads to raw checkpointer reads when the agent factory is unavailable (delta gate still applies; delta mode has no fallback) - deps: override websockets==16.0 (langgraph-sdk 0.4.2's <16 pin silently downgraded 16.0 -> 15.0.1; pin is not grounded in any API incompatibility) and bump the langchain lower bound to what the lockfile actually resolves * fix(checkpoint): include anchor checkpoint in degraded history walk + cover get_thread - _RawCheckpointReadAccessor.ahistory: alist(before=...) is exclusive while pregel's get_state_history treats config.checkpoint_id as the inclusive start; fetch the anchor explicitly so both read paths paginate identically - extend the degraded-path gateway test: GET /thread returns raw values, and POST /history with before starts at the anchor checkpoint * fix(gateway): preserve degraded checkpoint timestamps * fix(gateway): harden degraded checkpoint access * fix(gateway): resolve assistants for checkpoint reads --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com>	2026-07-22 08:33:29 +08:00
Ryker_Feng	8dafb667dd	fix(tui): derive /help text from the command registry (#4327) ... The /help string was hardcoded and had drifted from BUILTIN_COMMANDS, omitting six real commands (help, switch, resume, uploads, artifacts, details) and ordering the rest differently from the picker. Generate the command line from the registry so /help can never fall out of sync again, and add parity tests that guard against future drift.	2026-07-22 07:53:50 +08:00
RongfuShuiping	4bf028d048	feat(memory): add incremental agent-scoped Markdown fact storage (#4279) ... * feat(memory): add Markdown fact storage repository * docs(memory): explain storage rewrite for beginners * docs(memory): fix plan markdown formatting * refactor(memory): separate global summaries from agent facts * fix(memory): make Markdown fact updates incremental and safe * Update STORAGE_REWRITE_CHANGES.md * Delete docs/plans/STORAGE_REWRITE_PLAN.md * Delete docs/plans/STORAGE_REWRITE_CHANGES.md * fix(memory): address Markdown storage review feedback * fix(memory): complete review follow-ups * fix(memory): resolve storage review findings * feat(memory): add proactive Markdown migration CLI * fix(memory): harden Markdown storage concurrency * fix(memory): harden markdown storage migration * fix(memory): close migration review gaps --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> Co-authored-by: qin-chenghan <qinchenghan@Huawei.com>	2026-07-22 07:46:03 +08:00
Aari	3c0a45ad77	fix(skills): inject Langfuse metadata into the standalone skill scan (#4321)	2026-07-21 23:41:07 +08:00
dependabot[bot]	bfaa8cdfa3	build(deps): bump mcp from 1.27.0 to 1.28.1 in /backend (#4351) ... Bumps [mcp](https://github.com/modelcontextprotocol/python-sdk) from 1.27.0 to 1.28.1. - [Release notes](https://github.com/modelcontextprotocol/python-sdk/releases) - [Changelog](https://github.com/modelcontextprotocol/python-sdk/blob/main/RELEASE.md) - [Commits](https://github.com/modelcontextprotocol/python-sdk/compare/v1.27.0...v1.28.1) --- updated-dependencies: - dependency-name: mcp dependency-version: 1.28.1 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>	2026-07-21 23:35:02 +08:00
dependabot[bot]	907ffc6818	build(deps): bump pillow from 12.2.0 to 12.3.0 in /backend (#4350) ... Bumps [pillow](https://github.com/python-pillow/Pillow) from 12.2.0 to 12.3.0. - [Release notes](https://github.com/python-pillow/Pillow/releases) - [Changelog](https://github.com/python-pillow/Pillow/blob/main/CHANGES.rst) - [Commits](https://github.com/python-pillow/Pillow/compare/12.2.0...12.3.0) --- updated-dependencies: - dependency-name: pillow dependency-version: 12.3.0 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>	2026-07-21 22:44:19 +08:00
luo jiyin	d2116d861b	fix(sandbox): sync same-size E2B output updates (#4329) ... * fix(sandbox): detect same-size E2B output updates * test(sandbox): cover E2B output modification times * fix(sandbox): persist E2B output sync versions * test(sandbox): cover E2B output sync manifest * docs(backend): document E2B sync manifest * fix(sandbox): scope E2B sync manifests * test(sandbox): cover E2B sync manifest lifecycle * docs(backend): describe E2B manifest lifecycle * fix(sandbox): fall back from empty E2B sandbox ID * test(sandbox): cover empty E2B sandbox IDs * docs: clarify E2B output sync ownership	2026-07-21 22:30:14 +08:00
luo jiyin	da3feb3863	fix(sandbox): fail E2B bootstrap safely (#4325) ... * fix(sandbox): fail E2B bootstrap safely * test(sandbox): cover E2B bootstrap cleanup * docs(backend): document E2B bootstrap failure * fix(sandbox): discard E2B bootstrap failures * test(sandbox): cover E2B reconnect bootstrap failures * docs(backend): clarify E2B bootstrap recovery * fix: preserve E2B bootstrap errors * fix: reject falsey E2B bootstrap errors	2026-07-21 22:22:20 +08:00
Daoyuan Li	2bb22643ad	fix(feishu): check response.success() on send_file's reply/create calls (#4335) ... willem-bd's review on PR #4234 (Feishu card response.success() checks) flagged send_file (around lines 313-318) as having the same unchecked- response pattern: after _upload_image/_upload_file (which already raise on a business-level failure), the resulting file/image message.reply or message.create call's response was never checked, so a failed send logged "file sent" and returned True exactly like a real success. Adds the same response.success() check used by _reply_card/_create_card/ _update_card, raising (caught by the existing try/except, which already logs and returns False) so the caller can no longer mistake a silent business-level failure for a delivered file/image.	2026-07-21 18:37:10 +08:00
Andrew Chen	f113f10f36	fix(workspace-changes): count diff body lines starting with "-- "/"++ " (#4303) ... `_count_diff_lines` skipped every line starting with "+++ "/"--- " to drop difflib's two file headers. But a deleted hunk-body line whose content begins with "-- " (e.g. a SQL comment `-- get users`) becomes the diff line `--- get users`, and an added `++ ...` line becomes `+++ ...`, so those were dropped too — undercounting the user-visible +N/-M change summary (WorkspaceChangeSummary, per-file WorkspaceFileChange, and the run-event string in recorder.py). `difflib.unified_diff` always emits the two headers first, so skip them by position (`lines[2:]`) instead. Hunk `@@` lines start with `@` and are still ignored. Numeric/plain diffs are unaffected. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>	2026-07-21 17:48:04 +08:00
Ryker_Feng	fa496c0c8d	feat(browser): add agentic browser control (#4187) ... * feat(browser): add agentic browser control * fix(frontend): format browser view changes * fix(browser): keep browser optional and isolate sidecar layout * fix(browser): address PR review security and IME findings - Nginx: add a browser-stream WebSocket location before the generic /api/threads regex so Live upgrades instead of downgrading to HTTP (both nginx.conf and nginx.local.conf). - Ownership: require an existing owned thread for the WS stream and REST navigate, and tear down the browser session on thread deletion so a later caller cannot reuse a retained page/cookies by guessing the id. - SSRF: enforce the URL policy at the browser request boundary via a context-level route guard covering redirects, popups, iframes, and subresources (skipped for CDP-attached Chrome). - IME: skip key forwarding while a composition is active so confirming a CJK candidate with Enter no longer submits the remote page form. Adds regression tests for the request guard, session teardown on delete, and the composing-Enter key decision. * fix(frontend): smooth streaming in long tool threads * Revert "fix(frontend): smooth streaming in long tool threads" This reverts commit f0462516eabe77f138d4027ea1c714fb226683cf. * fix(browser): address review security and lifecycle findings - Reject cross-origin WebSocket upgrades on the live browser stream (Origin allow-list reuse of CORS/same-origin helpers) to close a WS-CSRF hole, and fail closed when the ownership store is absent. - Warn when a CDP-attached session runs with the SSRF request guard off, and drop the unreachable CDP screencast teardown dead code. - Read browser session launch config from a single canonical source (browser_navigate) so it is deterministic regardless of call order. - Bound per-thread Chromium accumulation with idle-timeout eviction and an LRU max-sessions cap. - Reset the Live reconnect counter on a successful open so the stream can't permanently stall after the cumulative attempt cap. * fix(frontend): reduce long tool thread render stalls Reuse stable historical message groups during streaming, defer heavy Markdown and browser previews, and lazy-decode message images. * fix(browser): keep live control responsive during continuous input Why: Manual browser control felt laggy — a physical click ran the remote Playwright click three times and each non-move input synchronously awaited a JPEG screenshot, so events queued behind capture (queue wait up to ~237ms). The first async attempt used a trailing-edge debounce, which froze the visible page until a wheel/keyboard gesture stopped ("scroll finishes, then it jumps"). What: - Frontend forwards one `click` per physical click instead of also emitting `down`/`up`, so the remote page is not clicked twice per gesture. - Backend detaches live-frame capture from input dispatch: non-move actions start a rate-limited background refresh loop (leading frame + bounded cadence) that keeps emitting frames while input continues and never blocks dispatch. - Add regression tests: input dispatch no longer awaits the screenshot, rapid inputs coalesce, and continuous input keeps refreshing before it stops. Scenarios: Verified in the live Browser panel — a single click completes in ~57ms (was blocked behind a 171ms capture), and a 1.14s sustained wheel gesture renders ~7 frames throughout the scroll instead of one frame after it ends. * fix(browser): harden worker and session lifecycle * fix(browser): address latest review feedback * fix(frontend): preserve optimistic new-chat message * test(e2e): preserve mocked message run ids * fix(browser): address capability review feedback --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com>	2026-07-21 11:46:33 +08:00
Daoyuan Li	b565e6c0f0	fix(workspace-changes): classify a symlink replacing a file distinctly from deleted (#4170) ... * fix(workspace-changes): classify a symlink replacing a file distinctly from deleted scan_workspace_roots() skipped every symlinked path entirely (host_file.is_symlink() -> continue), so the path was completely absent from a snapshot instead of being recorded as a metadata-only stub the way binary/large/sensitive-looking files already are. When an agent run replaces a tracked file with a symlink (e.g. rm config.txt && ln -s /some/other/path config.txt), the after-snapshot never contained that path at all, so compare_snapshots()'s _status() saw after_file=None and reported plain "deleted" -- silently hiding that the path is still alive on disk, now as a symlink that can point anywhere on the host, including outside the workspace root. Add a "symlink" classification mirroring the existing binary/large/ sensitive pattern: scan_workspace_roots() now records a symlink as a metadata-only FileSnapshot stub (symlink=True, symlink_target from os.readlink(), lstat'd without ever following the link) instead of omitting it. _status() reports "symlink_created" whenever a symlink newly occupies a path that was not already a symlink (brand new or replacing a prior file), so the security-relevant fact surfaces distinctly instead of collapsing into "deleted". A symlink genuinely removed with nothing replacing it is unchanged: still "deleted". Verified against a real POSIX symlink (WSL; native Windows symlink creation needs elevated privilege) driving the unmodified scan_workspace_roots()/compare_snapshots() functions, and via a patch-file revert/reapply cycle on this same fix to confirm the added regression tests fail before and pass after. * fix(workspace-changes): count symlink_created in the changed-file badge getChangedFileCount summed only created + modified + deleted, so a run whose only change was a symlink replacing a file (reported as the new symlink_created status, not deleted) produced a count of 0 and WorkspaceChangeBadge hid the badge entirely -- the exact scenario this PR targets, with the opposite of the intended result. Add symlink_created to the frontend WorkspaceChangeSummary interface (already emitted by the backend) and include it in the count. The per-file StatusIcon/statusLabel "modified" fallthrough is unaffected and left as a follow-up, per review. Regression test reverts cleanly to reproduce a count of 0 pre-fix. * fix(workspace-changes): rank symlink_created in file sort and complete the type contract sortWorkspaceChanges's statusRank had no entry for the new symlink_created status, so statusRank[left.status] - statusRank[right.status] evaluated to NaN for any comparison involving a symlink-created file, violating Array#sort's ordering contract instead of producing a deterministic order. The frontend WorkspaceChangeStatus and DiffUnavailableReason unions, and the WorkspaceFileChange symlink fields, also stayed narrower than what the backend now emits, so TypeScript's satisfies Record<...> guard on statusRank could not catch the gap. Widen WorkspaceChangeStatus to include "symlink_created" and DiffUnavailableReason to include "symlink", add the matching symlink/symlink_target_before/symlink_target_after fields to WorkspaceFileChange, and give symlink_created a rank alongside modified in statusRank -- restoring the satisfies guard's ability to catch a future unranked status. unavailableLabel now has an explicit "symlink" branch (new symlinkUnavailable i18n string, en-US + zh-CN) instead of falling through to the generic label. Also fixes the failing e2e-tests CI check: the existing workspace-changes.spec.ts mock summary predates the symlink_created field, so getChangedFileCount computed 1 + 1 + 0 + undefined = NaN and the badge rendered "Edited NaN files" instead of "Edited 2 files". Added symlink_created: 0 to the mock to match the real backend contract. New sortWorkspaceChanges unit tests revert cleanly against the unranked statusRank to reproduce the NaN-driven misordering. pnpm test (626 tests), pnpm check, and pnpm format are clean, and the previously-failing e2e spec plus the full e2e suite (94 tests) pass.	2026-07-21 10:22:55 +08:00
qin-chenghan	9eebc6a9e5	fix(config): sync _memory_config with AppConfig auto-reload (#4208) ... * fix(config): sync _memory_config with AppConfig auto-reload get_memory_config() now calls get_app_config() before returning the cached _memory_config singleton. get_app_config() auto-reloads when the config file signature changes, and its _apply_singleton_configs() calls load_memory_config_from_dict() to populate _memory_config. Without this side effect, changing memory.mode in config.yaml (e.g. from "middleware" to "tool") would never take effect at runtime because _memory_config retained the stale value. Only call get_app_config() when _app_config has already been loaded to avoid picking up a config file as a side effect of the first access to get_memory_config(), which would break callers that expect module-level defaults (e.g. unit tests). Catch FileNotFoundError for environments without a config file. Closes #4204. * fix(memory): broaden config-reload guard and add isolated-path tests Addresses two review findings from willem-bd: 1. Broken-config crash risk: broaden the except from FileNotFoundError to Exception so transiently broken config.yaml (YAMLError, ValidationError, ValueError) falls back to the last-good singleton instead of crashing per-turn hot paths (memory_middleware, summarization_hook, lead_agent/prompt.py). 2. Missing regression test: add test_get_memory_config_self_syncs_... (calls get_memory_config() after file mutation with no intervening get_app_config() call) and test_get_memory_config_falls_back_on_... (verifies broken config does not crash, returns cached value).	2026-07-21 10:14:29 +08:00
Daoyuan Li	8153e68eb8	fix(channels): escape Slack reserved chars before mrkdwn conversion (#4197) ... * fix(channels): escape &/</> in Slack outbound text before mrkdwn conversion Slack requires callers to replace &, <, and > with their HTML entity equivalents before sending message text, since an unescaped <...> triggers Slack's own mention/link syntax (e.g. <@USERID>, <http://url|label>). SlackChannel.send() ran msg.text through the markdown-to-mrkdwn converter and sent the result straight to chat_postMessage with no escaping step, so technical/code content like "if a < b && b > c:" would arrive as literal Slack markup and render broken or misinterpreted. Escaping must happen before the mrkdwn conversion, not after: the converter emits its own <url|label> syntax for real markdown links ([text](url)), and that generated syntax must reach Slack unescaped. Escaping raw input first (via html.escape(text, quote=False), which replaces & before </>) and leaving the converter's output alone satisfies both requirements. * fix(channels): preserve a line-leading > as Slack's blockquote marker _escape_slack_text's html.escape(text, quote=False) replaces every ">" with "&gt;", including a ">" at the start of a line -- Slack's own mrkdwn blockquote marker, which the converter otherwise passes through unchanged. Agent output using markdown blockquotes for quoting/callouts lost its blockquote styling and arrived as a literal "&gt; " prefix instead of a rendered blockquote. Only "&" and "<" neutralize Slack's "<...>" mention/link syntax; ">" is special to Slack only at the start of a line. Restore a line-leading ">" to a literal character after escaping (re.sub with MULTILINE), so the mrkdwn converter still renders it as a blockquote; a "<"/"&" anywhere, and a ">" that is not at a line start, still escape. New tests cover the line-start preservation, that the exemption does not widen into "never escape '>'" (a mid-line/non-leading ">" still escapes), and that restoration applies per line in multiline text; all three revert cleanly against the unconditional html.escape() to reproduce the blockquote-corruption bug. Full test_channels.py (259 tests) plus ruff check/format are clean.	2026-07-21 10:02:49 +08:00
ChenglongZ	ca16b64b26	feat(agent): support config-declared lead middlewares (#3964) ... * feat(agent): support config-declared lead middlewares * fix(agent): preserve configured extension middlewares * fix(agent): address middleware extension review * fix(agent): tighten middleware extension docs and tests	2026-07-21 09:39:44 +08:00
Tianye Song	8511fa6aa3	fix(memory): consolidated facts inherit expected_valid_days from sources (#4225) ... * fix(memory): consolidated facts inherit expected_valid_days from sources Consolidation (#3996) and per-fact expected_valid_days (#4143) were both authored by the same contributor but never connected: the consolidated new_fact carried the newest source's createdAt but no expected_valid_days, so _effective_fact_staleness_age fell back to the global staleness_age_days. A merge of several 200-day-old stable facts (each evd=3650) would land with no evd, read as a 90-day window, and re-enter the staleness candidate set on the very next cycle - the merge discarded the lifetime signal of the underlying information and contradicted consolidation's premise (these are stable, related facts worth synthesising). Fix: the merged fact inherits expected_valid_days set so it is re-reviewed at the EARLIEST source review deadline (min(createdAt + expected_valid_days) across sources, relative to the merged fact's createdAt = the newest source's). A merge combines details from every source, so a volatile sub-detail (evd=7) must not inherit a stable source's 3650-day window and escape staleness review for years - staleness KEEP/REMOVE is the only path that re-validates a merged fact, so biasing toward the soonest deadline keeps uncertain merges re-checked sooner. A source already past its deadline yields a minimal positive window (review next cycle) rather than the global fallback, which would defer an overdue review. Capped at the creation-time staleness_max_lifetime_multiplier like any new fact. Omitted when no source carries a valid evd (legacy facts fall back to the global age at read time, matching pre-feature behaviour). DRY: extract _read_expected_valid_days(fact) -> int | None, the shared type rule (int/float, reject bool, coerce to int BEFORE the > 0 guard) previously inlined in four places - _normalize_memory_update_fact, _effective_fact_staleness_age, the newFact creation cap, and consolidation inheritance. All four call the single helper. Coercing before the guard matters for values in (0, 1): 0.5 passes a raw > 0 check but truncates to 0, which would violate the helper's "positive int or None" contract; the order now matches the original _normalize_memory_update_fact rule. No prompt/schema change: consolidation's prompt does not surface source evd to the LLM, so asking the model to assign a merged lifetime would be guessing without signal. Source inheritance is deterministic and always available. Tests: consolidation evd cases now use time-stable createdAt (relative to now via a _days_ago helper) covering - earliest-deadline selection, creation-cap clamp, omit when no source evd, volatile source governs the deadline (and re-enters staleness next cycle), overdue source clamps to a minimal window, float coercion. Plus TestReadExpectedValidDays / TestEffectiveFactStalenessAge regression cases for the (0, 1) coercion-order fix. * fix(memory): reject non-finite expected_valid_days before int coercion The shared _read_expected_valid_days helper (introduced when consolidating the evd type rule across four call sites) coerces with int(raw) before the > 0 guard - reversing the original _normalize_memory_update_fact order so that a fractional 0.5 does not leak as 0. But int(raw) raises for non-finite floats: int(nan) raises ValueError and int(inf)/int(-inf) raise OverflowError. Python's JSON decoder accepts NaN / Infinity as floats by default, so a single malformed expected_valid_days in a hand-edited memory.json would abort staleness selection or consolidation instead of falling back to the global lifetime. On main, _effective_fact_staleness_age checked raw > 0 first, so NaN fell back safely (nan > 0 is false) - but inf did NOT (inf > 0 is true, so main also crashed on inf). This helper is now the persisted-fact read path for both staleness and consolidation, so the regression (and the pre-existing inf crash) must be closed here. Fix: require math.isfinite(float(raw)) before the int() coercion, then keep the existing positivity check and fallback. NaN / +/-inf all return None, so callers fall back to the global staleness_age_days. Normal int/float values (including large ones) are unaffected - isfinite is a no-op for them. Tests: - TestReadExpectedValidDays.test_rejects_non_finite_values - NaN, inf, -inf return None (not raise). - TestEffectiveFactStalenessAge.test_falls_back_for_non_finite_values - the persisted-fact read path returns the global age for each, no raise. - test_consolidation_with_non_finite_source_evd_does_not_raise - end-to-end: a NaN-evd source merged with a stable source does not abort consolidation; the NaN source's effective lifetime falls back to the global 90 and its deadline participates in the earliest-deadline computation. * test(memory): hoist _select_stale_candidates import + tidy deadline docstring Two review nits from the latest pass: - `_select_stale_candidates` was imported inline inside three test methods; hoisted to the module-level import block so the dependency is declared once. - `test_consolidated_evd_volatile_source_with_equal_created_at_future_deadline` had an abandoned calculation in its comment ("3 + 7 = 10 ... minus 3 already elapsed = 7? No:") that could mislead future readers into thinking elapsed-since-creation factors into the inherited window. Collapsed to a single clear line stating the window is relative to the merged createdAt, regardless of the source's current age. * fix(memory): reject huge-int expected_valid_days above timedelta.max.days _read_expected_valid_days routed every numeric value through float(raw) for the math.isfinite guard, but Python's JSON decoder parses an integer literal with no decimal point as an arbitrary-precision int (unlike 1e400, which decodes to float inf). So a hand-edited memory.json carrying "expected_valid_days": 10**400 raised OverflowError in float(raw) before math.isfinite was ever called - exactly the malformed-field-aborts-everything scenario the helper's docstring claims to prevent. The earlier non-finite fix only closed the float cases (NaN / +/-inf / 1e400). A huge int below the float limit but above timedelta.max.days (e.g. 10**12) would pass the helper and raise OverflowError downstream in timedelta(days=evd) during staleness selection or consolidation - the same crash fancyboi999 flagged for extend_by_days, just reached via a stored evd. Fix: branch on type so an int never passes through float() (matching the reviewer's suggestion), AND cap the returned int at timedelta.max.days (999999999) so the downstream timedelta(days=evd) call cannot overflow either. The float branch keeps the isfinite + int() coercion. Both branches share the 0 < evd <= timedelta.max.days positivity/range check. Normal values are unaffected - any legitimate expected_valid_days is far below the cap (the config ceiling staleness_max_extension_days tops out at 36500). Tests (all three layers): - TestReadExpectedValidDays.test_rejects_huge_int_above_timedelta_max - 10**400, 10**12, 10**9, timedelta.max.days+1 return None; timedelta.max.days itself is accepted. - TestEffectiveFactStalenessAge.test_falls_back_for_huge_int_above_timedelta_max - the persisted-fact read path returns the global age, no raise. - test_consolidation_with_huge_int_source_evd_does_not_raise[1e400|1e12|1e9] - parametrized end-to-end: a huge-int-evd source merged with a stable source does not abort consolidation; the bad source falls back to the global 90. * fix(memory): guard datetime arithmetic, not just timedelta construction The huge-int fix capped _read_expected_valid_days at timedelta.max.days, but that only proves timedelta(days=evd) can be constructed - adding it to a real fact timestamp still overflows datetime.max. @fancyboi999 reproduced it: a source with expected_valid_days=timedelta.max.days raises "OverflowError: date value out of range" at dt + timedelta(...) in the new consolidation deadline calculation. capping at timedelta.max.days was another patch chasing the next overflow boundary, not a real close. Root cause: the helper was doing datetime-range validation, but the safe bound depends on the datetime the evd is added to, not on the evd alone. So the responsibility moves to the arithmetic site, with try/except as the terminal guard - no concrete upper bound to be wrong about. Changes: - _read_expected_valid_days returns any positive int (huge ints included, not routed through float). Its job is type/positivity validation only. - New _safe_add_days(dt, days) -> datetime | None wraps dt + timedelta(days), returning None on OverflowError/ValueError. This is the terminal guard - there is no further boundary to overflow because try/except catches any datetime-range failure regardless of magnitude. - _select_stale_candidates uses _safe_add_days(now, -effective_age); a None result means the window is unrepresentably large, so the fact cannot yet be stale and is skipped (not selected). - Consolidation computes each source's deadline via _safe_add_days; a source whose deadline overflows falls back to the global staleness_age_days deadline (same treatment as a legacy no-evd source), so one malformed field cannot abort the merge. Normal values are unaffected - any legitimate expected_valid_days is far below the overflow boundary (the config ceiling staleness_max_extension_days tops out at 36500). Tests: - TestSafeAddDays: normal/negative shifts; 10**400/10**12/10**9 return None; timedelta.max.days (the exact reproduced value) returns None, not raises. - TestSelectStaleCandidates.test_huge_evd_does_not_abort_selection: a fact with a huge evd is skipped, not selected, and selection does not raise. - test_consolidation_with_huge_int_source_evd_does_not_raise now parametrized over [1e400, 1e12, 1e9, timedelta.max.days] - the last is the value that constructs a valid timedelta but overflows datetime arithmetic. - Helper/read-path tests updated to assert huge ints are returned as-is (the overflow guard is no longer in the helper).	2026-07-21 09:36:09 +08:00
Willem Jiang	1073393e07	Revert "fix(memory): follow config reload in get_memory_config (#4216)" (#4320) ... This reverts commit cd0f1e2212b0e910a5a3bd5f5e6f060b5bfe5cfa.	2026-07-21 09:34:04 +08:00
Yufeng He	cd0f1e2212	fix(memory): follow config reload in get_memory_config (#4216) ... * fix(memory): follow config reload in get_memory_config get_memory_config() returned the _memory_config singleton directly, and that singleton is only refreshed as a side effect of get_app_config() reloading (via _apply_singleton_configs -> load_memory_config_from_dict). A reader that reaches memory config without going through get_app_config() first -- e.g. the agent factory deciding whether to bind the memory tools via `feat.memory_config or get_memory_config()` -- therefore saw a stale memory.mode after a config.yaml edit, even though memory.* is documented as hot-reloadable. The agent kept the old mode until some unrelated get_app_config() call happened to refresh the singleton. Trigger the same signature-checked reload inside get_memory_config() so the singleton follows the config file, mirroring how the other config getters already resolve through get_app_config(). When no config file is on disk (tests, or a minimal deployment running purely on defaults / set_memory_config) get_app_config() raises FileNotFoundError; swallow it and fall back to the in-memory singleton so the accessor is never more fragile than before. Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com> * test(memory-config): pin malformed-config behavior for get_memory_config Address review feedback: the narrow except FileNotFoundError means a config file that exists but fails validation surfaces through get_memory_config() rather than being swallowed, consistent with every other unguarded get_app_config() caller. Add a regression test pinning that a malformed edit raises ValidationError and leaves the last-good singleton intact (not half-applied), and document the contract on the guard. Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com> --------- Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com>	2026-07-21 09:24:02 +08:00
hataa	92c8f2f03b	feat(authz): add built-in RBAC provider and provider factory (#4260) ... * feat(authz): add built-in RBAC provider and provider factory (Phase 1A-2, #4063) Phase 1A-2: RBAC provider + provider factory. No runtime behavior change. New authz/rbac.py — RbacAuthorizationProvider: - allow: '*' / True / list / [] / missing → deny-wins semantics - deny always overrides all allow forms - resource name explicit mapping (tool→tools, model→models, etc.) - unknown/missing role raises ValueError (never silent allow) - config compiled to immutable frozensets at construction - filter_resources preserves order, no mutation, consistent with authorize - sync == async decisions New authz/runtime.py — resolve_authorization_provider: - disabled → None (no import attempted) - enabled + no provider → ValueError - invalid class path / construction failure → ValueError with path - isinstance Protocol check post-construction - no caching, no fail_closed/default_role injection 48 tests (37 RBAC + 11 factory). No config schema change, no config_version bump. Per RFC #4063 Phase 1A-2. Layer 1/Layer 2 wiring deferred to Phase 1B. * fix(authz): reject unknown RBAC provider config Fail fast on misspelled top-level RBAC settings, cover factory error propagation, and record Phase 1B policy and audit caveats. * fix(authz): reject unreachable resource aliases Fail fast when RBAC config uses reserved request-side aliases, preserve same-name and custom resources, and add regression coverage for every mapped alias. * fix(authz): validate RBAC request identifiers	2026-07-21 09:23:14 +08:00
Aari	5eb59cb130	fix(sandbox): stop multi-worker orphan reconcile from killing peer sandboxes (#4221) ... * fix(sandbox): stop multi-worker orphan reconcile from killing peer sandboxes Docker sandboxes are shared across gateway workers, but each worker kept its own in-memory warm pool. Startup reconciliation adopted every running container, so a peer idle reaper could destroy sandboxes another worker still owned and tool calls hit 502 / Connection refused. Add file-based ownership leases under sandbox-leases/, only adopt true orphans, refuse idle/replica/shutdown destroy while a foreign lease is live, and renew the lease on create/get/release/reclaim. Fixes #4206 * fix(sandbox): close lease fail-open, hot-path IO, and check→destroy race Address review of the multi-worker orphan lease (#4206): - read_lease returns None only for a genuinely-absent lease and raises (CorruptLeaseError/OSError) when a lease is unreadable or corrupt, so the ownership check fails closed instead of mistaking an unprovable peer lease for a free container. clear_lease still removes a stuck/corrupt file. - get() no longer renews the lease (blocking mkdir/fsync/os.replace on the event loop path used by ensure_sandbox_initialized_async); active leases are renewed off the event loop from the idle checker (_renew_active_leases). - The ownership check and container stop run under a per-sandbox flock guard (lease_ownership_guard); every lease write takes the same guard so a peer's touch cannot interleave with a destroy. Same-host multi-worker scope, not a multi-pod distributed lock. Also fixes the ruff format lint on the branch. Adds regression tests: corrupt and unreadable lease fail closed, a tests/blocking_io anchor keeping get() non-blocking on the event loop, and a peer-touch/destroy interleave test. * fix(sandbox): share container ownership across gateway instances Rework of the #4206 fix per review: ownership state is shared through a third-party service instead of being maintained per gateway instance, following the stream_bridge precedent (sandbox.ownership.type: memory | redis). The file lease and its same-host flock guard are deleted, not ported — they only covered workers on one host, while the deployment that hits #4206 is a load-balanced multi-instance gateway. A lease answers "who reaps this container", not "who may use it". Containers are deterministic per (user, thread), so consecutive turns legitimately land on different instances: take() transfers ownership on acquire, while claim() gates every adopt/reap path. Leases carry a state — own: or del: — so a takeover is refused against a teardown in progress. Without it an unconditional take() would overwrite a destroyer's claim and the peer's container stop would land on a sandbox the new owner had already handed to an agent. renew() distinguishes a lapsed lease from one a peer took; only the latter drops the sandbox. Collapsing them meant a Redis restart evicted every in-flight sandbox on every instance at once. Renewal runs on its own thread with a TTL derived from its interval, never from idle_timeout: renewal used to ride the idle checker, which does not start at idle_timeout: 0, so leases silently lapsed on a supported config. Ownership establishment is fail-closed: a sandbox whose ownership cannot be published is never handed out, and a just-created container is destroyed rather than leaked as an adoptable orphan. Every destroy path claims before untracking. The memory store is single-instance only and says so; the resolver reads app_config.stream_bridge and the env var in the bridge's own order, so deployments already using Redis get a redis ownership store without extra config. * fix(sandbox): wait out a recovery grace before adopting a keyless container An absent ownership lease meant two opposite things on two paths. Renewal reads it as LAPSED and re-establishes it: nobody took the lease, so the container is still ours. Reconciliation read the same absent key as "orphan" and adopted on sight. After the store loses its keys (a Redis restart without persistence, or eviction under maxmemory) every owner is alive and merely pre-renewal-tick. Whichever instance reconciled first therefore adopted every live container; each real owner's next renewal reported LOST and dropped a sandbox it was serving mid-turn, leaving it for the adopter to idle-destroy — #4206 through the back door, in the very case the LAPSED handling was added to make safe. Not limited to startup: an already-running instance hits the same window from the idle checker's periodic reconcile. _adoptable_after_grace requires an untracked container to be seen unowned across a full lease TTL before it can be adopted. That rebuilds the delay the state loss erased: a live owner republishes within one renewal interval, shorter than the TTL by construction, while a crashed owner never does, so its containers are still adopted one grace later rather than leaking. A republished lease resets the grace; a pausing-only timer would still expire over a live owner's lease. The peek is read-only — the atomic claim still gates adoption. The grace is skipped when the store cannot coordinate across processes: no peer can hold a lease such a store would show us, so single-instance deployments keep instant orphan cleanup, and a grace could not help a multi-worker gateway on memory anyway. * fix(sandbox): hold the teardown lease for as long as the container stop runs claim(..., for_destroy=True) wrote the del: marker with the ordinary lease TTL and nothing refreshed it. renew() extends only own: and deliberately reports a teardown as LOST, and the destroy paths drop the sandbox from the maps the renewal loop iterates — so a container stop that outlived the TTL let the marker lapse, a peer's take() succeeded against the still-running container, and the stop then landed on the turn that had just been handed it. That is the exact window the del: state exists to close, reopened by its own expiry. The two lease states alone never made the per-sandbox flock redundant, as I claimed when deleting it: a held lock cannot expire, a lease can. The exclusion has to be held deliberately rather than assumed to outlast the work it guards. _held_teardown_lease wraps both _backend.destroy() call sites and re-claims the marker every renewal_interval_seconds until the stop returns. No store change is needed: claim(for_destroy=True) already refreshes an existing del: marker on both backends. Reachable without an abnormal backend. The schema bounds only renewal_interval_seconds (> 0) and ttl_multiplier (>= 2), so a legal config puts the TTL below a normal container stop; and LocalContainerBackend._stop_container passes no timeout to subprocess.run, so a wedged daemon blocks unbounded even at the default 120s TTL. The TTL stays finite on purpose: the heartbeat dies with the process, so a destroyer that crashes mid-stop still releases the container one TTL later instead of marking it undestroyable forever. * fix(sandbox): hold the teardown lease on every del: stop, and pin the claims that had no test 90936b49 said `_held_teardown_lease` wrapped "both" `_backend.destroy()` call sites. There are three. `_drop_unhealthy_sandbox` marks `del:` and then blocks on the same unbounded stop, and it untracks *before* claiming, so `_renew_owned_leases` cannot see the id either — nothing refreshed the marker. Reproduced against a real redis: the peer's `take()` succeeds 1.0s into a 2.5s stop. Same window, third path. That miss came from the habit the rest of this commit addresses: a property asserted in prose, with no test that could falsify it. Auditing every load-bearing claim in this feature — AGENTS.md, the store docstrings, the provider's design comments — against the test that would go red turned up several more, each verified by mutating the code and watching the suite stay green. Tests that could not fail: - `test_reconcile_fails_closed_when_ownership_unknown` reached the grace gate, not the claim. A bare MagicMock answers `owner()` with a truthy mock, so the container read as peer-owned and deferred; `claim()` was never called. It stayed green with `_claim_ownership` failing open. Adding the grace ahead of the claim is what hollowed it out — inserting a gate can silently disarm the tests for the gate behind it. - `test_adoption_grace_restarts_when_a_live_owner_republishes` never distinguished reset from pause. Those diverge only on a *second* lapse, which it never drove, so it passed with the reset deleted. Claims with no test at all, each now pinned (mutation → red, per test): - `destroy()`, `_evict_oldest_warm`, `_reclaim_warm_pool_sandbox`, `_register_created_sandbox` and `shutdown()`'s warm loop were each the one untested sibling of an "every path does X" enumeration. `shutdown()` was never driven with a non-empty warm pool, so a loop bypassing the ownership claim — stopping a live peer's container on our exit — went unnoticed. - Renewal's unknown-is-not-lost rule, the single deliberate exception to fail-closed. Inverting it drops every active and warm sandbox on every instance the moment the store blinks. - Both hops of the stream-bridge redis inference. Deleting either left the suite green while every config.yaml-native multi-instance deployment silently fell back to memory — #4206 reopened on exactly the deployments the inference exists for. Claims narrowed instead, because they promised more than the code delivers: - "run against both backends ... cannot drift" — CI provisions no redis, so the merge gate runs the memory tier only and the Lua never executes there. - "Every destroy path claims before untracking" — `_drop_unhealthy_sandbox` untracks first, deliberately, under its `expected_info` TOCTOU guard. - "Atomic: concurrent claims from different instances cannot both succeed" — true via Lua on redis, vacuous on the single-instance memory store, and pinned by neither, since the contract suite drives sequential calls. A concurrency test against the memory store would make the claim look covered while the mechanism that carries it still never runs in CI. * fix(sandbox): release the teardown marker when a destroy() stop fails The three `del:`-marked stop paths disagreed on failure. `_destroy_warm_entry` releases on both outcomes and says why: the stop failed, so the container is probably still up, and a marker left behind refuses its own thread's `take()` until the TTL lapses. `_drop_unhealthy_sandbox` does the same. `destroy()` had no such guard — a raising backend propagated straight past `_release_ownership`, and the thread could not re-acquire for a full TTL. Fails safe rather than fatal: a stuck marker stops peers from touching the container, it is not the cross-instance kill. But the paths must agree, and this one is the odd one out. Release, then re-raise. Swallowing would be the easier symmetry with `_destroy_warm_entry`'s `return False`, but `destroy()` has no failure return and `shutdown()` logs per sandbox off the exception, so swallowing would silently narrow what callers can see. Found by comparing the three paths after @fancyboi999 asked for release to be handled "consistently with the other destroy paths" on the unhealthy path — which 0d2377b2 already does. This is the sibling that wasn't. * fix(deploy): bump chart config_version to 27 for sandbox.ownership config.example.yaml went to 27 with the new sandbox.ownership section, but the chart embeds its own copy and stayed at 26, so validate-chart failed. A bare bump: the chart already sets stream_bridge.type=redis, which is what resolve_ownership_config infers a redis ownership store from, so no field change is needed. * fix(sandbox): release the teardown lease from its heartbeat, not the caller `_held_teardown_lease` joined its heartbeat only briefly and the caller cleared the `del:` marker right after the stop. A refresh `claim` still in flight (`RedisOwnershipStore` had no socket timeout, so a round trip could block) could land *after* that release and rewrite `del:` on a container whose stop had already completed — refusing a fresh `take()` (or rolling back a fresh create) until the TTL. Move the release into the heartbeat's own `finally`, after its loop stops, so no refresh can run after it. The three destroy paths no longer release after the `with` (`destroy()`'s no-container branch still does, since no lease was held there). Bound every store round trip with a socket timeout so the in-flight refresh — and thus the deferred release — stays finite, and broaden the heartbeat's `except` so an unexpected error cannot strand the marker during a long stop. Also fold in the review follow-ups: stop re-resolving an already-resolved ownership config in the factory, document the Redis-outage-vs-TTL boundary in config.example.yaml, and add a tests/blocking_io anchor pinning that `release()`'s store round trip stays off the event loop. * fix(sandbox): refuse a non-destroy claim that would unwind our own teardown `claim(for_destroy=False)` against our own `del:` lease fell through and overwrote it with `own:`, cancelling a teardown that was already in flight. The container stop cannot be recalled, so downgrading the marker would let a `take()` hand out a container that is about to die -- #4206, self-inflicted. No caller does this today: the two non-destroy callers run against an absent key (the LAPSED re-claim) or an unowned one (post-grace reconcile). The contract has to forbid it rather than rely on that staying true. Fixed in both backends. The redis rule lives in Lua and the memory rule in Python, so fixing one only would let them drift silently -- and the shared contract suite is what is supposed to catch that drift, so it now covers this. Also adds a contention test for `claim`. The suite drove sequential calls only, so it pinned the exclusion predicate but not the atomicity that predicate depends on; eight instances now race for one container and exactly one must win. * fix(sandbox): bound the container stop so it cannot outlive its teardown lease `_stop_container` passed no `timeout` to `subprocess.run`, so a wedged container runtime blocks it forever. The `del:` marker is what keeps a peer from re-acquiring the container while the stop runs, but a marker is a lease and a lease can lapse: a store outage longer than the TTL frees it, a peer's `take()` succeeds against the still-running container, and the stop then lands on the turn that was just handed it -- the exact #4206 failure. The teardown heartbeat already covers the case where the store stays reachable. This bounds the worst case independently of the ownership layer, which is the point: it holds even when the ownership layer is the thing that failed. A timeout is not swallowed like a `CalledProcessError`. That error means the runtime answered "I could not stop it"; a timeout means we do not know, and the container is probably still running -- returning normally would let `_destroy_warm_entry` report a clean stop and drop the warm entry, leaking a running container nothing tracks. * fix(sandbox): exclude this instance's own reapers from its acquire path An ownership lease excludes peers and nothing else. `claim()` and `take()` both succeed against our own `own:` lease by design -- that is what lets a destroy path claim what it already owns -- so `del:` says nothing to this process's other threads. Meanwhile every reaper decides outside `_lock`, because a store round trip must not be held under the lock that guards every acquire. So each reaper acts on a decision its own acquire path may already have invalidated, and the store cannot see the difference. Six paths end in an irreversible act (a container stop, or closing a host-side client) on a decision made outside the lock. All six reproduce: _evict_oldest_warm re-checks warm membership, then releases the lock _reap_expired_warm no re-check at all _cleanup_idle_sandboxes re-verifies idle, then releases the lock _renew_owned_leases acts on a stale renew() -> LOST release() same staleness on its own refresh _drop_unhealthy_sandbox untracks before claiming, opening discovery Both warm reapers are a regression from the deferred pop this branch introduced: `WarmPoolLifecycleMixin` popped under the lock, so a reclaim's membership check failed and the race could not occur. Deferring the pop is still right (popping first loses the container on a refused claim), so the exclusion has to be made explicit instead. The idle path is pre-existing in shape, but this branch widened it from a few instructions to a network round trip by claiming ownership before untracking. Two guards, because the two directions want opposite answers: Reaping -- nothing may promote it. The reaper reserves the id, and every promote path refuses a reserved id exactly as it refuses a peer's `del:` (drop and cold-start). The "is this still reapable?" test travels with the reservation as a predicate and runs in the same critical section, because checking first and reserving second is the window, not a narrower version of it. Forgetting -- the peer legitimately wins, so the promote is what to detect. `_publish_ownership` bumps a per-id acquire epoch; the callers that decide from a store round trip snapshot it first, and the pop is skipped if it moved. Object identity cannot substitute: the reuse path re-publishes ownership while handing out the same tracked `AioSandbox`, so an identity check sees nothing and the pop closes a client mid-turn. `still_reapable` is required rather than defaulting to unconditional -- the safe default is the one that makes a new call site think about it. That diverges from the mixin hook, which is safe because this provider overrides both mixin callers, and loud rather than silent if those are ever dropped. Also closes a client leak on the discover path: "nothing to roll back" was true of the container but not of the HTTP client constructed before the publish, which the sibling create path already closes. The shared-store test view rebound `owner_id` outside the store's lock, so a concurrent claim could execute under the wrong id and read its own lease as a peer's. Serialized, so the heartbeat-hold tests stop flaking. * fix(sandbox): mark acquire intent before the ownership round trip A guard must become visible no later than the transition it guards. The acquire epoch cannot manage that for `take()`: the takeover is durable before `take()` returns -- redis has committed the SET while the reply is still in flight -- and the epoch can only be written afterwards. In that interval the store already says the container is ours while the epoch still reads as it did when a renewal decided `LOST`, so the stale forget walks through, drops the maps and closes the client the acquire is about to hand back. Acquire then returns an id the provider no longer tracks and `get()` answers `None` for the rest of the turn. `_publish_ownership` now publishes an intent mark under `_lock` before the round trip; the epoch keeps covering the other half, "an acquire completed since you decided". `_forget_lost_sandbox` honours the intent mark unconditionally rather than only when an epoch is supplied -- today's epoch-less callers cannot reach the window, but "no epoch" reading as "no guard" is how the next caller of a dangerous primitive gets written. The same invariant had four more instances, all reproduced: reuse returns a decision the forget already invalidated -- before the mark is set a `LOST` is both current and correct, so the forget legitimately runs and the entry reuse decided to hand out is gone. Re-check after publishing and fall through to discovery instead. reclaim installs an entry a reaper reserved after its check -- the warm entry is still visible during the stop, and the reaper's claim succeeds because reclaim's own take() just made the lease ours. Re-check likewise. the reservation was released before the entry was removed -- the pop belonged to the caller, leaving a gap where the container is stopped, the entry is still in `_warm_pool`, and nothing marks it. `_destroy_warm_entry` removes it itself, inside the reservation; the pop stays deferred relative to the stop, just not to the reservation. reconcile adopts a container this instance is tearing down -- adoption is a promote and needs the same reservation check as the others. Neither existing guard excludes it: the claim succeeds because the lease is ours, and on `memory` the recovery grace is skipped outright. The pre-round-trip checks in reuse and reclaim are kept as early-outs, since they skip a health check and a store round trip on a doomed entry, and are pinned to that job rather than to a correctness role they no longer hold. The teardown reservation predicate runs under `_lock`, so it must not touch the lock. Documented rather than engineered around: making the lock reentrant to tolerate it would trade a loud hang for a quiet class of re-entrancy bugs across the rest of the provider. * fix(sandbox): honor local teardown after ownership publish * fix(sandbox): clear a stale warm entry when an id becomes active Active and warm are exclusive states, and the two register paths were the only place that could hold both: they inserted into `_sandboxes` without popping `_warm_pool`, so one container ended up with two reapers. `_reap_expired_warm` judges an entry by its warm timestamp and never consults `_last_activity`, so it stops a container an agent is actively using while `_sandboxes` still hands out its client. Reachable because `_reconcile_orphans` adopts an untracked-but-running container into the warm pool inside the register's publish -> track window, and on the `memory` store it adopts on sight: `_adoptable_after_grace` short-circuits when `supports_cross_process` is False, so an id carrying this process's own lease reads as adoptable. That window is new to this branch -- on main the track was a single locked insert with nothing before it. Both register paths now pop the warm entry inside the same locked section that installs the active one. * fix(sandbox): harden ownership renewal teardown --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com>	2026-07-21 09:09:40 +08:00
Matt Van Horn	3949340610	fix: harden Postgres async engine with pool_recycle and command_timeout to stop stale-connection 504s (#4230) ... * fix: harden Postgres async engine with pool_recycle and command_timeout to stop stale-connection 504s * fix(persistence): make postgres command timeout a configurable database setting Default the app-ORM command timeout to 30s (below nginx's 60s proxy deadline), expose it as database.command_timeout with null to disable, and add regression coverage. Addresses P1 review feedback. Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> * fix(persistence): decouple DB command timeout from nonexistent proxy deadline The command timeout bounds stalled ORM queries independently; drop the incorrect coupling to a 60s nginx deadline (actual proxy timeout is 600s). * feat(config): make pool_recycle configurable Expose pool_recycle alongside command_timeout and pool_size in config, example config, and the helm chart, keeping the 300s default, per review. --------- Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com>	2026-07-21 08:52:31 +08:00
VectorPeak	16a77cb780	fix(serper): ignore malformed image URLs (#4319) ... Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>	2026-07-21 08:15:33 +08:00
Willem Jiang	ac5fd46281	feat(backend): bound LLM call concurrency and shed burst-rate (429) retries (#4294) ... * Create a feature of Process-global LLM concurrency cap * Added configuration of llm_call of max_concurrent_calls * Classify limit_burst_rate and expose retry params via config.yaml * refactor(middleware): encapsulate LLM concurrency state in a dataclass Address PR #4294 review feedback (github-code-quality bot): the bare module-level globals _GLOBAL_CONCURRENCY_LOOP / _GLOBAL_CONCURRENCY_LIMIT were flagged as unused - a false positive, since both are read on the recreate condition, but the `global`-declaration pattern tripped the analyzer. Replace the three globals + `global` declaration with a single _ConcurrencyState dataclass singleton mutated in place. Behavior is unchanged (lazy recreate when the running loop or configured limit changes); the state is now co-located and no longer relies on bare globals. dataclasses is already an established harness convention. Co-Authored-By: Claude <noreply@anthropic.com> * fix(middleware): make LLM concurrency limiter process-wide + jitter burst retry Addresses PR #4294 review (fancyboi999, CHANGES_REQUESTED) - two P1 issues. P1 #1: the asyncio.Semaphore limiter was loop-bound, so it recreated per event loop and the cap was NOT process-wide: lead-agent calls (main loop) and subagent calls (the isolated persistent loop in subagents/executor.py) each got their own semaphore, and the sync graph path (wrap_model_call) bypassed the cap entirely. Recreating on loop/limit change also abandoned permits held by the prior instance. Replace it with a _ProcessWideLimiter built on threading primitives (not loop-bound): one limiter shared across every event loop and both sync/async wrappers. The cap is mutable via set_limit (never recreates, so in-flight permits are never abandoned); permits release in finally and async waiters unregister on cancellation, so cancellation never leaks capacity. Wire it into wrap_model_call (sync) too - previously a direct handler() call. P1 #2: the first (and only) burst-rate retry was deterministic at 5000ms. prev_delay_ms was seeded from the 1000ms normal base, so for burst the window collapsed to randint(5000, max(5000, 1000*3)) = randint(5000, 5000) - a fleet that failed together realigned on the same 5s tick. Seed the first retry from the reason-specific base (prev_delay_ms=None on loop init) so the burst window is [burst_base, cap] = [5000, 8000], non-degenerate. Retry-After is still honored verbatim. Tests: rename semaphore tests -> limiter; add an autouse fixture resetting the process singleton; add regressions the reviewer asked for - cross-loop (lead + isolated-loop subagent), two concurrent sync calls, limit-change while a permit is held (same instance, permit preserved), cancellation no-leak, and burst first-retry non-degeneracy with default config (real and seeded RNG) plus a concurrent de-synchronization case. Verified the burst guard goes red on the old logic ({5000}) and green on the new. Co-Authored-By: Claude <noreply@anthropic.com> * Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> * Potential fix for pull request finding 'Statement has no effect' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> * fix(middleware): lossless limiter handoff, generation-aware cap, burst-rate CB gate Address the open P1/P2 review findings on #4294: - P1 #1 (cancellation handoff): reserve the permit for a specific waiter at dequeue time (grant-at-dequeue, _AsyncWaiter.granted) so a waiter cancelled in the post-dequeue / pre-reacquire window hands its reservation to the next waiter (_handoff_granted_permit_locked) instead of stranding it. No cancellation window remains. - P1 #2 (hot-reload generation): move cap updates out of the per-attempt path; give the limiter one generation-aware owner (set_limit_if_newer with a monotonic instance seq proxy for config freshness) so a stale in-flight run cannot rewrite a freshly-lowered cap. max_concurrent_calls is now genuinely hot-reloadable, resolving the reload-boundary inconsistency by option (b) - no STARTUP_ONLY_FIELDS change (retry params truly hot-reload). - P2 (circuit breaker): gate _record_failure on reason != "burst_rate" so burst-rate (limit_burst_rate) exhaustion - a transient slope-throttle, not "provider down" - does not trip the CB and fast-fail ALL calls. - P3: clamp the jitter window to the cap before drawing (uniform spread instead of piling at the cap); document the per-process / GATEWAY_WORKERS cap semantics in config + the field description. Tests: add the reviewer-requested regressions (cancel-after-dequeue handoff; stale-instance-doesn't-overwrite-lowered-cap across sync + isolated-loop async; burst_rate-exhaustion-doesn't-trip-CB sync + async). Each is red on the prior buggy logic and green on the fix. _build_middleware now routes llm_call knobs through AppConfig so __init__ applies the cap. 71 middleware tests pass; 212 across the blast radius (1 pre-existing skip). Co-Authored-By: Claude <noreply@anthropic.com> * fix(middleware): startup-only LLM concurrency cap; report effective retry budget Addresses review feedback on #4294 (fancyboi999 CHANGES_REQUESTED on acfc7617): P1 - the generation guard measured construction order, not config freshness, so a stale AppConfig(cap=3) constructed after a fresher AppConfig(cap=1) could restore the higher cap; and on a downscale 3->1 release() handed excess permits to queued waiters, keeping in_flight pegged at the old cap. Replace the pseudo-generation path with a startup-only cap: the first middleware __init__ resolves and freezes the cap; later instances (newer or older config) are no-ops. No runtime cap mutation means no downscale race and no freshness/construction-order race. Per-call gate is now `limiter is None` only, so a reloaded instance with max_concurrent_calls=0 cannot silently drop the frozen cap. Removes _owner_seq / set_limit_if_newer / _grant_to_queued_locked / _next_instance_seq; file 946 -> 926 lines. P2 - burst-rate calls are capped at 2 attempts but the retry log line, the llm_retry stream event max_attempts, and the user-facing message still used self.retry_max_attempts (3), so the frontend showed 1/3 then stopped after attempt 2. Thread the effective max_attempts (_max_attempts_for) through the logger, _emit_retry_event, and _build_retry_message. Also: document max_concurrent_calls as startup-only in the config field description and config.example.yaml (prose only - the startup-only: prefix is top-level AppConfig-field granularity and would mislabel the otherwise hot-reloadable llm_call section / break the reload_boundary drift test). Rewrite the cap-mutation tests for startup-only semantics; add P2 retry-budget event tests (sync+async, teeth-verified red on the bug); fix bot nits (empty except blocks -> gather(return_exceptions=True); bare await statements -> assigned+asserted). Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>	2026-07-21 08:07:49 +08:00
Aari	e66f455d51	fix(skills): don't treat a lazily evaluated PEP 695 type alias as a network sink (#4315) ... * fix(skills): don't treat a lazily evaluated PEP 695 type alias as a network sink * test(skills): cover type alias parameter bounds	2026-07-21 00:02:16 +08:00
Ryker_Feng	24a45a4e68	feat(tui): add clear command (#4306)	2026-07-20 23:56:37 +08:00
Daoyuan Li	6544d96cc4	fix(skills): close AST literal-only shell=True bypass in SkillScan (#4057) ... * fix(skills): close AST literal-only shell=True bypass in SkillScan SkillScan's Python analyzer only classified a subprocess call as the CRITICAL, hard-blocked python-shell-exec rule when its shell= keyword was the literal AST constant True. Any non-literal value with the same runtime effect - a variable (shell=shell_flag) or an expression (shell=bool(1)) - fell through to the HIGH, non-blocking python-subprocess classification instead, silently bypassing enforce_static_scan's deterministic CRITICAL gate despite behaving identically to shell=True at runtime. _call_has_shell_true is renamed to _call_shell_may_be_true and now fails closed on ambiguity: any shell= value that is not a literal, statically-provable False is treated as CRITICAL, matching the literal shell=True case. A call with no shell= keyword at all is unaffected (subprocess already defaults to shell=False). Adds regression tests for the variable and expression bypass shapes, plus a boundary test locking in that literal shell=False remains a non-blocking warning. * fix(skills): fail closed on **-unpacked shell= in SkillScan _call_shell_may_be_true only checked keyword.arg == "shell", so a subprocess.* call that supplies shell via **-unpacking (a keyword node with arg is None) fell through to the non-blocking python-subprocess classification instead of the CRITICAL python-shell-exec path. Treat any **-unpacked keyword as shell-ambiguous and fail closed, same as the existing shell=variable/shell=expression handling. This intentionally over-blocks a **-unpack that carries no shell key, since a mapping's contents are not knowable by static analysis; that tradeoff is documented inline and covered by a dedicated test.	2026-07-20 23:50:47 +08:00
Aari	09e25b8a32	fix(auth): let deployments close local self-registration (#4311) ... * fix(auth): let deployments close local self-registration The OIDC provisioning policy (allowed_email_domains, require_verified_email, auto_create_users) is enforced only in the SSO callback via get_or_provision_oidc_user. POST /api/v1/auth/register creates a local account without consulting any of it, and nothing can turn that path off, so a deployment declaring an email-domain allowlist can still be joined by any address through local registration. Add auth.local.allow_registration (default true, so existing deployments are unchanged) and gate /register on it before the account is created. Report the flag from /setup-status so the login page stops offering a signup entry the Gateway will reject. /initialize is deliberately not gated: it is the bootstrap path, guarded by admin_count == 0, and closing it would leave a fresh install unable to create its first admin. An unreadable config.yaml falls back to the pre-gate default (open) rather than making these two endpoints a hard dependency on the file. * docs(auth): align registration-gate fallback wording with the FileNotFoundError catch	2026-07-20 23:33:09 +08:00
Aari	1a1c5def0d	fix(agents): classify web_fetch error pages as errors, not successful evidence (#4314) ... * fix(agents): classify web_fetch error pages as errors, not successful evidence * fix(agents): classify 502/503/504 error shells as transient, not internal Per review: a gateway error page is the try-a-different-source case, so 502/503/504 reason phrases now map to transient (warn, then escalate) while 500/501 stay internal (stop). This mirrors _ERROR_RULES' own split and removes the two phrases that classified differently through the shell table than through _classify_error_text ("service temporarily unavailable", "gateway timeout") — now pinned by a cross-path consistency test over the live table plus a chain-level test that a 503 page warns and escalates instead of blocking web_fetch on first sight. Also per review, _classify_error_shell returns a copy of the category attrs instead of the rules-table dict, matching _classify_error_text. Dropped "gone" from the phrase table: a single-word reason phrase collides with legitimate one-word document titles ("# Gone"), and 410 error pages are rare while such titles are not; negative controls record the decision. * test(agents): pin crawl4ai's recorded renderer shape; note web_capture asymmetry Per review: the title rule assumes every web_fetch provider renders title-first. Eight of the nine providers hold by construction (f"# {title}" or the shared Article.to_markdown()); crawl4ai renders server-side, so its generator output (crawl4ai 0.9.2, fit and raw) was measured over the same corpus and recorded as fixtures driven through the real tool and middleware chain. A comment on _PAGE_CONTENT_TOOL_NAMES records that web_capture's absence is intentional: its dead-target signal is the provider warning suffix, which belongs to the provider boundary.	2026-07-20 23:05:47 +08:00
Aari	b02308bb1d	fix(dingtalk): drop inbound messages that carry no conversation identity (#4316)	2026-07-20 23:00:44 +08:00
Aari	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>	2026-07-20 08:18:23 +08:00
Daoyuan Li	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.	2026-07-20 08:01:18 +08:00
Andrew Chen	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>	2026-07-20 07:54:15 +08:00
lllyfff	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>	2026-07-20 07:24:34 +08:00
Aari	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	2026-07-20 07:04:59 +08:00
Daoyuan Li	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.	2026-07-19 22:36:15 +08:00
Daoyuan Li	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.	2026-07-19 22:12:30 +08:00
Daoyuan Li	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.	2026-07-19 22:08:48 +08:00
Daoyuan Li	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.	2026-07-19 20:08:52 +08:00
Aari	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>	2026-07-19 20:01:52 +08:00
Hanchen Qiu	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.	2026-07-19 18:56:03 +08:00
luo jiyin	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	2026-07-19 18:45:52 +08:00
Mohammed Ansari	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>	2026-07-19 18:26:26 +08:00
Aari	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.	2026-07-19 17:24:31 +08:00
Andrew Chen	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>	2026-07-19 17:13:44 +08:00
Andrew Chen	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>	2026-07-19 17:01:28 +08:00