mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-07-29 17:36:00 +00:00
130 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
ly-wang19
|
4582a41347
|
fix(skills): offload blocking filesystem IO in update_skill and serialize writes (#3565)
* fix(skills): offload update_skill's blocking IO and share the config write lock Re-applied on top of main rather than merged: update_skill has since gained admin gating, user-scoped storage and a PUBLIC vs CUSTOM/LEGACY split, so the offload is applied per path. - PUBLIC: the extensions_config.json read-modify-write (path resolve, snapshot, merge, write, reload) moves to a worker thread via asyncio.to_thread. The payload is built from a snapshot so the cached singleton is never mutated in place while the write is still in flight. - CUSTOM/LEGACY: set_skill_enabled_state is offloaded; the non-user-scoped fallback takes the same shared-file RMW path as PUBLIC. - Both load_skills calls (and storage construction) are offloaded. The RMW lock now lives next to reload_extensions_config as get_extensions_config_write_lock() and is acquired by both the skills router and the MCP router, which performs the same read-modify-write on the same file. Previously each router held its own module-local lock, so once both sides offloaded, a PUT /api/mcp/config could run inside a skill toggle's read->write window and the later write would silently drop the other's change. The lock is keyed by the running event loop rather than being a module-level singleton: asyncio primitives bind to the first loop that awaits them, which makes a plain module-level lock unusable in a process that runs more than one loop. Adds a cross-router regression anchor asserting a skill toggle and an MCP update never overlap inside the RMW (max in-flight 1); it observes 2 when the routers use separate locks. * fix(config): own the extensions_config RMW lock from the worker thread An asyncio.Lock held around `await asyncio.to_thread(...)` protects only the awaiting task. If that task is cancelled the context manager releases the lock immediately while Python keeps running the worker thread, so a second skills or MCP writer could acquire it and operate on extensions_config.json concurrently with the first worker — reopening the lost-update window this was meant to close. The per-event-loop keying had a second hole: writers on different loops got different locks and so did not exclude each other at all. Replace it with a process-wide threading.Lock acquired *inside* the worker that performs the RMW (`_write_extensions_skill_state` and `_apply_mcp_config_update`), so ownership belongs to the thread doing the writing and is held until the write and reload actually finish, regardless of what happens to the caller. A threading.Lock also has no event-loop affinity. Adds a cancellation regression: the skills worker is paused inside the lock, its route task is cancelled, and the MCP writer is started — the MCP RMW must not enter until the skills worker is released. Against the previous asyncio-lock design this test fails with ['skills-enter', 'mcp-enter']. The two existing serialization tests instrumented by replacing the worker functions, which now bypasses the lock under test; they instead patch inside the real workers (each module's reload_extensions_config, the last step under the lock) so the production lock is exercised. --------- Co-authored-by: ly-wang19 <ly-wang19@users.noreply.github.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
阿泽
|
9c7cd4cad3
|
feat(sandbox): add thread data mount override for upload sync (#4536) | ||
|
Vanzeren
|
c48de5e70b
|
feat(checkpoint): make delta snapshot_frequency configurable (#4516)
* feat(checkpoint): make delta snapshot_frequency configurable * fix(config): carry legacy checkpoint_delta_snapshot_frequency with warning Addresses review on #4516: the rename from the flat database.checkpoint_delta_snapshot_frequency key to nested database.checkpoint_delta.snapshot_frequency silently dropped the old value (pydantic extra="ignore"). Add a before-validator that maps the legacy key onto the nested one with a deprecation warning (nested key wins when both are set), plus a CHANGELOG breaking-change note covering the rename and the 1000 -> 10 default change. * fix(checkpoint): validate frozen snapshot frequency |
||
|
Ryker_Feng
|
0a9ce5d7e3
|
feat(suggestions): configure follow-up suggestion count (#4533) | ||
|
Zhipeng Zheng
|
838037188e
|
feat(channels): share inbound webhook dedupe across pods via Postgres (#4210)
* feat(channels): share inbound webhook dedupe across pods via Postgres (#4120) * ci: run cross-pod inbound dedupe integration tests in CI Expose the job Postgres service via DEDUPE_TEST_POSTGRES_URL so the integration tests (issue #4120) actually execute instead of silently skipping. Normalize the URL for asyncpg (postgresql:// -> +asyncpg, drop libpq-only sslmode) and await the now-async _is_duplicate_inbound in test_github_dispatcher. |
||
|
March-77
|
b22f85c686
|
fix(sandbox): reconcile E2B sandboxes safely (#4443)
* fix(sandbox): reconcile E2B sandboxes safely * fix(sandbox): clear failed E2B adoption intent |
||
|
Vanzeren
|
e01173d8b2
|
bench(checkpoint): production-shaped full/delta benchmark with configurable snapshot frequency (#4467)
* feat(checkpoint): production-shaped full/delta benchmark with configurable snapshot frequency - Group benchmark scripts into per-family folders (checkpoint/, sandbox/) - Extract shared benchmark infrastructure into checkpoint_bench_common.py - Add checkpoint_delta_snapshot_frequency config (default 1000, process-frozen); freeze it in make_lead_agent and DeerFlowClient; key the state-schema adaptation cache by resolved frequency - New bench_production.py: per-case child processes run N ainvoke turns through the real lead-agent graph (scripted deterministic model, real AsyncSqliteSaver), then measure GET /state + POST /history through the real Gateway route stack in one event loop (httpx ASGITransport), cold/warm accessor-cache split, cross-mode digest gates - New summarize_production.py: delta/full ratios plus decision metrics (snapshot_write_spike, cache_effect_ms, checkpoint_write_share, auto-discovered history per-limit ratios) * fix(checkpoint): address production benchmark review |
||
|
Ryker_Feng
|
7aa314b4c1
|
feat: add Lark CLI integration (#3971)
* feat: add lark cli integration * fix: polish lark integration actions * feat: support lark incremental permissions * fix: detect lark authorization completion * fix: harden lark integration install * feat: expand lark auth scopes and reuse host auth in sandbox Default lark auth to least-privilege (recommend=false, base sign-in only) and expose the full set of lark-cli --domain business domains as native --domain grants instead of a 4-domain read-only mapping. Resolve the skill pack from the latest larksuite/cli GitHub release at install time with content-hash integrity, and surface version/runtime drift in status. Share the per-user lark-cli config/data profile between the Gateway Settings auth flow and agent conversations by mounting the integration dirs into the AIO sandbox and injecting the matching env for lark-cli commands, with an allowlisted extra_mounts path in the provisioner/K8s backend and traversal guards on integration paths. * style: fix lint issues from ruff and prettier Sort imports in the provisioner PVC test and re-wrap two long i18n description strings to satisfy backend ruff and frontend prettier CI. * fix(lark): address managed integration review feedback * fix(frontend): stabilize integrations settings e2e * test(sandbox): isolate remote backend legacy visibility check * test: fix backend unit failures after merge * Harden Lark integration review fixes * Format Lark integration E2E test * fix(lark): harden sandbox credential exposure and status disclosure Address willem_bd's security review on PR #3971: - Mount the per-user lark-cli config dir (long-lived appSecret) read-only into the AIO sandbox; only the refreshable-token data dir stays writable. - Redact host filesystem paths (install_path, cli.path) from GET /lark/status and the config/auth complete responses for non-admin callers, fail-closed on any auth error. - Document the npm postinstall trade-off (--ignore-scripts is not viable because @larksuite/cli fetches its platform binary in postinstall). - Document the sandbox credential trust boundary in AGENTS.md and README, pointing at the sidecar-broker follow-up (#4338). --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Aari
|
37c343fe30
|
fix(summarization): summarize with the run model, fall back on summary-provider failure (#4361)
* fix(summarization): own the run model for compaction; bound failure With summarization.model_name: null the summary model resolved to config.models[0] while the executing model is selected per run; when they differ and models[0]'s provider is broken (expired key, quota, outage) compaction silently failed every triggered turn and context grew unbounded until the main provider 400s the run (#3103's shape), even though the run's own model was healthy. Model ownership is now sourced from the builders, not re-derived at runtime: - The lead, subagent, and manual /compact builders each pass the resolved run model into create_summarization_middleware(run_model_name=...). The middleware no longer reads runtime.context / get_config(), which do not carry a custom agent's or a subagent's resolved model, so a custom-agent lead run and a distinct-model subagent now summarize with their own model, not models[0] / the parent's. Runtime re-resolution and the per-name model cache are removed. - model_name: null summarizes with the run's own model; an explicitly configured summary model generates and falls back to the run model on failure. The fallback is built lazily after the primary fails and its construction is guarded, so a broken fallback cannot skip a healthy primary or escape the automatic failure boundary. Failure is bounded and side-effect-safe: - An empty or whitespace-only response is treated as a generation failure, not a valid summary, so compaction never removes all history for an empty replacement. - compact_state/acompact_state take raise_on_failure independent of force: the manual /compact path always surfaces a generation failure (even force=false) and routes it to the existing ContextCompactionFailed path (HTTP 500 -> frontend error toast) instead of an unconsumed response reason. The automatic path leaves compaction state unchanged. - before_summarization hooks fire only after a replacement summary exists. SummarizationConfig.model_name, config.example.yaml, and docs/summarization.md document the final lead/subagent/manual ownership rules. Part of RFC #4346 (section A). Evaluating fraction/triggers against the run model's profile (profile ownership) is a separate follow-up. * fix(summarization): manual /compact model ownership + fail-open construct/parse Manual /compact carried only agent_name, so it derived the run model from the custom-agent model or config.models[0] and missed the request-selected model the run path uses (request -> custom-agent -> default). Carry model_name through ThreadCompactRequest and the frontend compact call, resolve with the same precedence, and move the custom-agent config read off the event loop (asyncio .to_thread) with user_id so the strict blocking-IO gate is not bypassed by the broad except. Make one summary attempt own its full lifecycle so the fail-open boundary covers construction and response parsing, not just invocation: build each candidate model lazily and guarded (a raising constructor falls through to the healthy run model instead of breaking agent construction), build the model_name:null primary from the run model rather than config.models[0], and run response text extraction inside the invocation try so a failing .text accessor falls back instead of escaping compaction. Adds factory-level constructor-failure, response-extraction-failure (sync/async), and route-path model-ownership tests. |
||
|
luo jiyin
|
3b77a7401b
|
fix(sandbox): enforce E2B replica capacity limits (#4391)
* fix(sandbox): enforce E2B replica capacity limits (in-process) Add SandboxCapacityExceededError with diagnostic fields. Add overflow_policy (wait/reject/burst), acquire_timeout, and burst_limit config options. Implement atomic capacity reservation with a four-slot model: reserved / active / warm / transitioning. Transitioning slots close the window where active-to-warm or warm-to-active transitions appear to have zero occupied slots, which would let concurrent acquires exceed the configured replica ceiling. Re-route release, reclaim, and evict through transitioning counters. Add shutdown guard: reject waiters, kill VMs created during shutdown. Add 14 tests: policy enforcement, release+acquire race, warm-reclaim race, shutdown-waiter interaction, shutdown-during-create, and concurrent different-thread capacity assertion. Related: #4339 * fix: harden e2b sandbox capacity lifecycle * fix: retain e2b capacity during uncertain eviction * fix: serialize e2b tombstone eviction * fix: retain capacity after uncertain e2b cleanup * fix: track e2b remote operations during shutdown * fix(sandbox): validate E2B capacity config * fix(sandbox): classify capacity errors * fix(sandbox): harden E2B capacity lifecycle * test(sandbox): cover E2B review findings * docs(changelog): note E2B capacity behavior * docs(readme): explain E2B overflow handling * docs(backend): record E2B lifecycle rules * docs(sandbox): clarify destructive E2B reset * fix(sandbox): close E2B capacity race gaps --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
黄云龙
|
126fc9ea81
|
fix(subagents): clamp subagent limit consistently with MIN_SUBAGENT_LIMIT (#4081)
* fix(subagents): align prompt and middleware subagent limit; allow min of 1 SubagentLimitMiddleware clamped max_concurrent to [2, 4] internally, but agent.py and client.py fed the raw config value into the system prompt, so a user-configured 1 (or 5) produced a prompt that disagreed with the enforced middleware limit. Lower MIN_SUBAGENT_LIMIT to 1 and clamp the raw config value with _clamp_subagent_limit() at both the agent factory and the embedded client so the prompt and middleware see the same value. * fix: remove unused imports MAX_CONCURRENT_SUBAGENT_CALLS, MIN_CONCURRENT_SUBAGENT_CALLS, clamp_subagent_concurrency * fix: harmonize clamp range [1,4] across middleware, config, and prompt path; fix lint - Changed MIN_CONCURRENT_SUBAGENT_CALLS from 2 to 1 so prompt.py's clamp_subagent_concurrency and the middleware's _clamp_subagent_limit both clamp to [1,4] — eliminating the divergence where the prompt told the model 'max 2 task calls' but the middleware enforced 1. - Applied _clamp_subagent_limit at build_middlewares (agent.py:360) so all 3 construction sites (agent.py:360, agent.py:450, client.py:259) consistently clamp the config-resolved limit. - Derived MIN_SUBAGENT_LIMIT / MAX_SUBAGENT_LIMIT from MIN_CONCURRENT_SUBAGENT_CALLS / MAX_CONCURRENT_SUBAGENT_CALLS so the two module-level definitions stay in sync. - Added TestConfigParity.test_prompt_path_and_middleware_clamp_agree regression test. - Fixed lint. * fix(lint): add missing imports for MIN_CONCURRENT_SUBAGENT_CALLS and MAX_CONCURRENT_SUBAGENT_CALLS * docs+test: update AGENTS.md clamp range to 1-4; add prompt/middleware parity regression test - backend/AGENTS.md still documented the old [2,4] clamp in two places; updated to [1,4] to match MIN_CONCURRENT_SUBAGENT_CALLS = 1. - Added test_apply_prompt_template_single_subagent_limit_matches_middleware: renders the real system prompt with max_concurrent_subagents=1 and asserts the advertised HARD LIMITS value equals SubagentLimitMiddleware's enforced max_concurrent — the end-to-end check that would have caught the [1,4] vs [2,4] prompt-path divergence flagged in review. * refactor: simplify per review — restore clamp delegation, drop redundant call-site clamps Per willem-bd's review, reduce the PR to the one behavioral change plus docs/tests: - _clamp_subagent_limit delegates to clamp_subagent_concurrency again instead of inlining a byte-identical copy; with a single source of truth the TestConfigParity sync-check class is unnecessary — dropped. - Revert the call-site clamps in agent.py (build_middlewares, _make_lead_agent) and client.py (_ensure_agent) to main: both downstream consumers (SubagentLimitMiddleware.__init__ and the prompt path) already clamp internally, and the cross-module private import of _clamp_subagent_limit goes away with them. - Keep MIN_CONCURRENT_SUBAGENT_CALLS = 1 (the fix), the [1, 4] docstring updates, the AGENTS.md range corrections, and the end-to-end prompt/middleware parity test for single-subagent mode (docstring reworded: on main a configured 1 was bumped to 2 by both paths — there was no divergence to fix, just a silently raised floor). * test: fix stale comment referencing reverted agent.py/client.py call-site clamps --------- Co-authored-by: nankingjing <nankingjing@users.noreply.github.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Aari
|
0d4d0cb17d
|
feat(agents): database-backed storage for custom agent definitions (#4359)
* feat(agents): database-backed storage for custom agent definitions Add an agent_storage.backend switch (default file, behaviour-unchanged) with a db backend that stores each custom agent as a row in the shared SQL persistence layer, so a multi-instance deployment sees the same agents on every node (#4331, #4357). Introduces an AgentStore interface routing all read/write surfaces, an agents table + migration 0006, startup validation, and a file->db importer. Follows the thread_meta store / run_events backend-switch / 0003_scheduled_tasks migration patterns; no new dependency. * fix(agents): make db storage path production-ready (review round 1) Addresses review feedback on the db/sync agent-storage path: - sql.py: mirror the async engine's per-connection SQLite PRAGMAs on the sync engine (busy_timeout=30000, synchronous=NORMAL, foreign_keys=ON, WAL) so both engines behave identically against the shared DB; guard the engine cache with a lock (double-checked) so concurrent first-touch cannot build duplicate engines or register the connect listener twice. - routers/agents.py + routers/assistants_compat.py: offload the sync-store reads that ran on the event loop (list/get/check, update's pre-read + legacy guard + refresh, and assistants_compat's four list routes) via asyncio.to_thread — on db+postgres each was a network round trip stalling the loop. Writes were already offloaded. - file.py: translate the create() mkdir(exist_ok=False) race FileExistsError into AgentExistsError (router 409, matching SqlAgentStore's IntegrityError path); correct the _write docstring — per-file atomic replace, two commits sequential not transactional. Tests: sync-engine PRAGMA + engine-cache reuse assertions; file create-race -> AgentExistsError; strict Blockbuster anchor over the read endpoints so a regression back onto the loop fails CI. * fix(agents): address round-2 review on the db store path - update_agent tool: align the docstring/inline comment with FileAgentStore._write. Cross-field write atomicity is db-only; the file backend commits config then soul via two sequential os.replace (a crash between them can leave a fresh config.yaml beside a stale SOUL.md). The dropped partial-write *reporting* is an intentional tradeoff — the stage-then-replace safety is preserved (test_update_agent_soul_failure_does_not_replace_config still holds). - SqlAgentStore.update(): true upsert. Catch IntegrityError on the insert-on-missing branch, re-fetch and apply, so two concurrent first-time writes (e.g. two setup_agent handshakes) converge instead of surfacing a raw UNIQUE(user_id, name) violation as a 500. Symmetric with create(). - get_agent_store(): document the graph-subprocess config-resolution invariant (the except->file fallback is a genuine no-config path, not a mask for a misconfigured graph process) and pin it with two tests driving the real get_app_config() file resolution: db resolves from an on-disk config.yaml, file fallback when config is unresolvable. * test(agents): cover SqlAgentStore.update() write-race upsert recovery Mandatory-TDD test for the round-2 fix in 0680340a: two concurrent first-time update()s where the loser's insert hits UNIQUE(user_id, name). Deterministically forces the IntegrityError recovery path by making the first _row probe miss the committed winner, and asserts last-writer-wins instead of a surfaced 500. |
||
|
Daoyuan Li
|
09d9cf53d2
|
fix(harness): add timeout to invoke_acp_agent to prevent indefinite hangs (#4238)
invoke_acp_agent had no timeout anywhere in its call path, and ACPAgentConfig had no timeout field. If the ACP agent subprocess answers initialize/new_session correctly but then hangs inside prompt(), the tool call - and therefore the whole agent turn - blocks indefinitely, with the child process left running. MCP stdio servers already guard against this class of hang via tool_call_timeout; ACP agent invocations had no equivalent. Add ACPAgentConfig.timeout_seconds (default 1800, ge=1), mirroring the shape/default of subagents.timeout_seconds, and wrap the conn.prompt() call in asyncio.wait_for(). On TimeoutError, return a clear error instead of hanging; exiting the spawn_agent_process context block triggers the ACP library's own graceful-then-forceful subprocess cleanup, so the hung process is actually terminated. |
||
|
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. |
||
|
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>
|
||
|
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). |
||
|
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 |
||
|
Willem Jiang
|
1073393e07
|
Revert "fix(memory): follow config reload in get_memory_config (#4216)" (#4320)
This reverts commit cd0f1e2212b0e910a5a3bd5f5e6f060b5bfe5cfa. |
||
|
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> |
||
|
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> |
||
|
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> |
||
|
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> |
||
|
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 |
||
|
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. |
||
|
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. |
||
|
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>
|
||
|
HaotianChen616
|
756eac0d1a
|
feat(tool):Add structured synopsis for oversized tool output previews (#3377)
* Improve tool output preview synopsis * Add JSON path anchors to tool output synopsis * Fix JSON synopsis line anchors * fix(synopsis): tighten detectors and fix CSV first-row join Address review feedback from @willem-bd on PR #3377. Detectors: - _looks_yaml now requires >=3 key-shaped lines and refuses bare uppercase-tag lines ('INFO: ...', 'ERROR: ...') that look like log lines and would round-trip into a flat string dict via safe_load. Previously a 200-line log file was classified as 'YAML object with 3 top-level keys' and lost every line, count, and middle signal. - _try_yaml refuses payloads that safe_load collapses to a dict of all strings (the shape tracebacks and log lines collapse into). - _try_table applies the header-must-look-like-identifiers and minimum-row-count guards only to TSV, since the same safeguards would reject legitimate small CSVs. Refuses tab-indented bash output, ls -l listings, and tree dumps. Rendering: - CSV first data row is now rendered as a key=value list joined by ' | ' (e.g. 'name=Ada | description="a fine, brilliant logician" | score=98'). The previous delimiter.join(rows[1]) silently re-split cells that contained the delimiter inside a quoted cell, which made the synopsis report a 3-column table as 5 columns and misled the model about column count and content. Text summary: - _summarize_text now omits the closing excerpt entirely when the input is shorter than 2 * _TEXT_EXCERPT_CHARS, since the previous opener/closer slices overlapped and duplicated text for short inputs (build_tool_output_synopsis is reachable directly from tests and other callers that pass small inputs). Tests: - Update test_table_preview_extracts_columns to assert the new key=value list format. * fix(synopsis): drop JSON path line/byte offset hints The path-location hint was computed by string-searching for the quoted key in the original content and reporting its byte offset and line number. This anchors at the first textual occurrence of the key string, which is wrong when the key also appears as a value earlier in the document, or when the same key recurs at multiple depths. With nested paths the anchor drifts further on every step because the search cursor is advanced past each previous match. Concrete cases: content = '{"label": "items", "items": {"id": 1}}' _json_path_location(content, ['items']) -> ' (line 1, byte offset 10)' # the value, not the key content = '{"data": {"info": 1, "data": {"info": 2}}}' _json_path_location(content, ['data','data','info']) -> ' (line 1, byte offset 30)' # the inner first 'info', not the second The synopsis instructed the model to 'Start near the line hints above when present', so a wrong anchor would send read_file into the wrong region of the persisted .tool-results file. Drop the hint entirely. The path itself ('$.data.items') is already useful navigation; the agent uses read_file with start_line based on its own judgement of where the relevant slice is. Tests: - Update test_json_preview_reports_nested_paths to assert no 'line ' or 'byte offset ' appears in the body before the Access section. - Rename test_json_line_hints_use_original_content_offsets to test_json_paths_are_emitted_without_line_hints and invert the assertions to check the hints are absent. * fix(synopsis): bound _scalar_examples recursion depth Mirror the _JSON_STRUCTURE_DEPTH cap used by _json_container_paths and _json_shape so that deeply nested JSON cannot trigger RecursionError inside build_tool_output_synopsis. In ToolOutputBudgetMiddleware.awrap_tool_call the synopsis is built inside asyncio.to_thread(_patch_result, ...); a RecursionError would surface as a tool-call failure and the user would lose the entire output. 300-level nested JSON is well inside what an attacker-controlled MCP tool, a JSON-RPC-over-JSON-RPC chain, or a buggy serializer can produce. * feat(synopsis): restore inline raw head/tail sample The synopsis-only preview silently dropped the raw head/tail bytes that preview_head_chars / preview_tail_chars used to inline. For text/code/log outputs the agent lost first/last KB of the actual content and had to issue a follow-up read_file round-trip to see the trailing region (last paragraph of a fetched article, final error line in a traceback, closing diagnostics of a bash run). Restore an inline 'Raw sample (head + tail)' section in the preview. The section is composed by slicing head_chars from the start and tail_chars from the end of the content (with a '...' separator between them, and the tail suppressed when it would overlap the head). For binary-like output, the synopsis's own sample is reused unchanged. This makes preview_head_chars / preview_tail_chars operational again for every kind except binary, which already had a sample channel. Tests: - Rename test_json_preview_extracts_structure_instead_of_head_tail to test_json_preview_includes_structure_and_raw_sample and assert the raw sample section is present and the payload is reachable in the head slice. * test(synopsis): add regression tests for willem-bd review findings Add 8 regression tests under TestToolOutputSynopsis, one per finding in @willem-bd's review of PR #3377: - test_review_5_log_lines_are_not_misclassified_as_yaml Pins the YAML detector to refuse 'LEVEL: message' log lines. - test_review_6_json_paths_are_emitted_without_byte_offset Pins the removal of byte/line hint from JSON path descriptions. - test_review_7_scalar_examples_respects_depth_cap Pins that 500-deep nested JSON does not raise. - test_review_8_csv_first_row_quoted_cells_round_trip Pins the new key=value list format for CSV first-row rendering and asserts that quoted cells with embedded delimiters survive. - test_review_9_tsv_detector_rejects_tab_indented_bash Pins that tab-indented bash output is not classified as TSV. - test_review_10_preview_includes_raw_head_and_tail_sample Pins the restored inline 'Raw sample (head + tail)' section. - test_review_11_short_text_does_not_duplicate_excerpts Pins that closer is suppressed for inputs shorter than 2 * _TEXT_EXCERPT_CHARS. - test_review_12_preview_head_tail_chars_are_operational Pins that head_chars / tail_chars are wired into the rendered preview and not silently dropped. Also removes the now-stale 'byte offsets are approximate anchors' sentence from render_tool_output_preview's Access block; the synopsis no longer emits byte/line hints, so the guidance to 'start near the line hints' was misleading. * fix(synopsis): resolve lint errors on tool output budget tests Local 'make lint' on feat/tool-output-synopsis-preview (after fast-forward to current main) failed with three errors in tests added by PR #3377: - E501: 307-char bash_out literal in test_review_9_tsv_detector_rejects_tab_indented_bash - E741: ambiguous single-letter 'l' in test_review_11_short_text_does_not_duplicate_excerpts - E741: same ambiguous 'l' on the closing assert Replace the long literal with a join of per-row entries, rename the loop variable from 'l' to 'ln', and run ruff format on the two touched files to absorb the formatting drift introduced by the merge with main. Verification: - make lint -> All checks passed; 643 files already formatted - pytest tests/test_tool_output_budget_middleware.py -> 110 passed * fix: address willem-bd review findings (code/csv misclassification, text duplication, line snapping, dead constant, xml hardening, depth consistency) - _CODE_HINTS: require stronger signals for use/fn (trailing ; or parenthesised) - _try_table: apply _TABLE_MIN_DATA_ROWS gate to CSV too (not just TSV) - config.example.yaml: correct misleading comment about preview_head/tail_chars - _summarize_text: skip opener/closer excerpts when raw sample will be appended - _build_raw_sample: snap to line boundaries for clean truncation - Remove dead constant _TABLE_FIRST_ROW_CHARS - Prefer defusedxml for XML parsing (billion-laughs protection), fallback to stdlib - Replace _json_shape magic number 2 with named _JSON_SHAPE_MAX_DEPTH constant - Update tests to match new CSV gate (>=5 rows) and line-snapped sample counts * style: ruff format fix for tool_output_synopsis.py and test_tool_output_budget_middleware.py * fix(tool-output): address 4 review comments - DoS hardening + size cap 1. XML entity-expansion DoS: skip _try_xml when defusedxml is not available (SafeET is None), falling through to text + raw sample. (cid=3587721336) 2. YAML alias-bomb DoS: refuse to parse YAML content > 500 KB. (cid=3587721340) 3. Unbounded content parse: add _MAX_SYNOPSIS_INPUT_BYTES=5MB cap; oversized output falls back to raw head/tail sample instead of full parse. (cid=3587721346) 4. Scalar examples surface mid-document values: add docstring note that the synopsis is a structural summary, not a confidentiality filter. (cid=3587721353) * fix(tool-output): ruff format the synopsis string to one line --------- Co-authored-by: qinchenghan <qinchenghan@huawei.com> |
||
|
Huixin615
|
65afc9b1d2
|
fix(skills): apply allowed-tools only to active skills (#4098)
* fix(skills): scope allowed-tools to active skills * fix(skills): tolerate stale active skill paths * chore: retrigger CI * fix(skills): document policy activation limits * perf(skills): reuse per-step tool policy decisions * fix(skills): harden runtime tool policy contracts * fix(skills): redact cached policy decisions * fix(skills): make slash tool policy authoritative * fix(skills): preserve policy-safe discovery tools * test(skills): cover explicit task delegation policy |
||
|
Tianye Song
|
8da7cbf028
|
feat(memory): LLM-assigned per-fact expected_valid_days and staleFactsToExtend (#4143)
Re-ports this feature onto the pluggable-memory backend introduced in #4122 (the original #4143 was force-pushed clean by accident and auto-closed). The #4122 refactor moved the staleness logic into the self-contained DeerMem backend (backends/deermem/deermem/core/) and reverted it to the pre-feature global-threshold version, so the per-fact lifetime work is re-applied here against the new module layout + DI (MemoryUpdater is now (config, storage, llm)-injected; config lives on DeerMemConfig, not host MemoryConfig). **expected_valid_days (creation)** The LLM assigns a per-fact review window when storing each new fact. The prompt exposes five tiers (<=14 d transient -> >365 d very stable). The value is capped at write time by staleness_age_days x staleness_max_lifetime_multiplier (default 20.0 -> 1800 d ~= 5 years; range 1.0-100.0) so the model cannot set an initial lifetime so long the fact is never re-evaluated. The default 20.0 makes the "> 365 d very stable" tier achievable out of the box (3.0 silently clamped it to 270 d). **staleFactsToExtend (review)** During staleness review the LLM can emit extension entries for kept facts whose window seems miscalibrated. new_evd = min(days_since_created + extend_by_days, staleness_max_extension_days). Extensions use an absolute ceiling (default 3650 d ~= 10 years; range 90-36500) rather than the creation multiplier - they are deliberate review decisions that must be able to advance the window beyond the initial cap, but the absolute bound prevents timedelta overflow (a model-supplied extend_by_days of 10**9 previously crashed every later candidate-selection pass with OverflowError) and LLM misfire. **Invariant correctness** - Read-time cap removed from _effective_fact_staleness_age; cap is write-time only so extensions actually advance the review window. - proposed_remove_ids hoisted out of the removals sub-block and used to exclude from extension, so a cap-surviving proposed-removal fact is never extended. - extend_by coerced to int before the > 0 guard (a fractional 0.9 would pass the float check then int() to 0, silently writing a zero-delta extension). - days_since uses total_seconds() // 86400 (not .days truncation). - staleness-section html.escape uses quote=False to match the prompt.py convention; only <, >, & break element-text structure. **Tests** test_memory_staleness_review.py was module-level skipped by #4122 ("full unit-test migration is a follow-up"). This PR performs that migration: DI construction via (DeerMemConfig, _FakeStorage), _build_staleness_section back to the (candidates, config) signature, plus new coverage for per-fact selection, EXTEND with the absolute cap, the overflow next-cycle regression, the proposed-removal-not-extendable case, fractional extend_by skipping, and the creation-time cap. 67 tests, all green. |
||
|
Tu Naichao
|
959bf13406
|
fix(memory): flush memory queue on graceful shutdown to prevent loss (#4181)
* fix(memory): bounded shutdown flush via MemoryManager.shutdown_flush Re-applies the memory-queue shutdown drain on top of the pluggable MemoryManager abstraction (#4122): the old top-level MemoryUpdateQueue singleton is gone, so the drain is now a backend contract instead of host code reaching into the queue. - MemoryManager ABC: shutdown_flush(timeout) -> bool. Every backend implements a bounded graceful-shutdown drain. - DeerMem: queue.flush_sync (daemon-thread + Event.wait hard timeout for the uninterruptible sync LLM call; joins an in-flight worker first so contexts a debounce Timer already pulled out are not lost on exit; skips inter-item sleep on the drain path; per-item succeeded/failed count), exposed via shutdown_flush. - noop: shutdown_flush is a clean no-op success. - Gateway lifespan: call get_memory_manager().shutdown_flush(timeout) after channels/scheduler stop, via asyncio.to_thread, try/except bounded. No host-level pending/processing guard -- the backend short-circuits on an idle buffer, so the host cannot "forget" the in-flight case (structurally eliminates the guard race flagged on the prior revision). - shutdown_flush_timeout_seconds added to the shared MemoryConfig (host-owned lifecycle budget, default 30, 1-300) + exposed on MemoryConfigResponse and the embedded client; config_version 25 -> 26. Tests: queue flush_sync (7), lifespan drain incl. False-branch caplog assertion + disabled gate (3), ABC contract noop/deermem (3). * fix(chart): gateway grace period so memory drain is not SIGKILLed K8s defaults terminationGracePeriodSeconds to 30s, shorter than the Gateway's graceful-shutdown work (channel stop ~5s + memory queue drain default 30s). Without an explicit grace period, K8s SIGKILLs the memory drain mid-flight and silently re-introduces the loss shutdown_flush is fixing (flagged on the prior revision). - gateway pod: terminationGracePeriodSeconds (default 45, configurable). - gateway container: preStop sleep (default 5, 0 disables) so the Service/ingress deregisters the pod before SIGTERM begins the drain. - values.yaml + README: both configurable; README documents that the grace period must track memory.shutdown_flush_timeout_seconds. * docs(memory): document shutdown_flush_timeout_seconds + lifespan drain Add the host-shared field to the memory config list and Config Schema summary in backend/AGENTS.md, noting the lifespan drain and the K8s grace-period relationship. * fix(chart): bump embedded config_version to 26 The chart's embedded `config:` block (values.yaml + README example) still had config_version: 25 after commit f3ca8e9f raised config.example.yaml to 26, failing the validate-chart config_version drift check. Bump both to 26. |
||
|
qin-chenghan
|
ad45f59d66
|
feat(memory): pluggable memory abstraction with self-contained DeerMem backend (#4122)
* feat(memory): pluggable + self-contained memory system (MemoryManager plan phases 1 & 2) Phase 1 — Pluggable (steps 0-10): - ABC MemoryManager (9 methods) + singleton factory + drop-in backend discovery - DeerMem default backend with core/ (storage/queue/updater/prompt/message_processing) - NoopMemoryManager backend (proves pluggability) - All call sites (middleware/hook/prompt/gateway/client/app) routed through manager - hasattr capability probing for DeerMem-internal methods (no hard imports) - MemoryConfig gains manager_class field; shared vs DeerMem-private annotated Phase 2 — Self-contained DeerMem (steps 11-18): - backend_config passthrough + DeerMemConfig (all DeerMem-private fields moved off MemoryConfig) - DI: DeerMem owns storage/queue/updater/llm as instance attributes (no global singletons) - Storage independence: core/paths.py with own root (~/.deermem or ), factory auto-injects deer-flow's runtime_home() as absolute base_dir (zero-config) - LLM independence: core/llm.py via langchain init_chat_model (no create_chat_model) - Trace independence: optional tracing_callback replaces inject_langfuse_metadata/request_trace_context - Message processing independence: hide_from_ui default-skip + optional should_keep_hidden_message hook - Internal imports → relative (only deer_mem.py ABC import is host-relative) - Carrier (deer_mem.py adapter) / portable (deermem/ config+core) split - New tests: test_deermem_self_contained + test_memory_manager_pluggable; all memory tests migrated - Other-agent demo: samples/other_agent_demo/ + automated portability test - config.example.yaml memory section updated to phase-2 schema * feat(memory): port consolidation + staleness fix into self-contained DeerMem; phase-2 host hooks Port upstream #3996 (memory consolidation) and #3993 (staleness KeyError fix) from origin/MemoryManager into the pluggable, self-contained DeerMem structure (backends/deermem/deermem/), adapted to the DI MemoryUpdater (config injected, not get_memory_config globals): - DeerMemConfig: add consolidation_enabled (opt-in, default false) / consolidation_min_facts / consolidation_max_groups_per_cycle / consolidation_max_sources - prompt.py: factsToConsolidate JSON field + {consolidation_section} placeholder + CONSOLIDATION_PROMPT constant - updater.py: _coerce_source_confidence / _select_consolidation_candidates / _build_consolidation_section module helpers (matching the existing _select_stale_candidates style); consolidation normalization in _normalize_memory_update_data; consolidation apply in _apply_updates (after max_facts trim, with apply-time guardrails mirroring staleness); staleness KeyError fix (f["id"] -> f.get("id") is not None) applied to both the staleness guardrail and the consolidation allowed_source_ids comprehension - config.example.yaml: consolidation section under memory.backend_config - tests/test_memory_consolidation.py: 40 DI-adapted tests (running, not skipped) incl. the staleness KeyError regression Also includes in-flight phase-2 host-integration work: storage_path semantics (any absolute/relative value = root dir) and host-default tracing_callback / should_keep_hidden_message hooks injected into backend_config by the factory. Co-Authored-By: Claude <noreply@anthropic.com> * feat(memory): add noop backend template and backends guide - backends/noop/: complete drop-in template (config.py with zero deer-flow imports, noop_manager.py with a 6-step new-backend walkthrough in its docstring, commented optional fact-CRUD capabilities). - backends/README.md: which files to touch when adding/swapping a backend, the 5-item backend contract, and common pitfalls. - manager.py: generalize backend examples in comments (drop mem0-specific references). Co-Authored-By: Claude <noreply@anthropic.com> * fix(frontend): guard formatTimeAgo against invalid timestamps Return a neutral placeholder when the input date is invalid (e.g. an empty lastUpdated from a backend with no memories) instead of throwing 'Invalid time value' from date-fns. Co-Authored-By: Claude <noreply@anthropic.com> * feat(memory): wire tool-driven memory mode through the MemoryManager ABC tools.py (memory_search/add/update/delete) now calls get_memory_manager() instead of the removed host memory module, so tool mode (memory.mode: tool) works for any backend. DeerMem.search is implemented (case-insensitive substring match, ranked by confidence) as a stand-in for the planned semantic retrieval; noop.search returns [] (unchanged). Fact-CRUD tools use getattr+callable probing -- backends lacking those ops (noop) get a clear JSON error instead of crashing. Tests: test_memory_tools rewired to mock the manager (handler tests) + TestModeGating retained; test_memory_search now covers DeerMem.search; pluggable stubs test updated (search no longer a stub). Co-Authored-By: Claude <noreply@anthropic.com> * fix: resolve lint errors (import sorting, type annotation quotes, E402 in skipped tests) * docs: restore explanatory comments in config.example.yaml memory section * fix(security): port html-escape memory facts fix (#4097) to vendored DeerMem prompt.py * fix(memory): address review + port dropped upstream memory fixes Review blockers (vendored DeerMem): - #4044 restore _escape_memory_for_prompt (current_memory blob in MEMORY_UPDATE_PROMPT) - prevents </current_memory> breakout - #4028 html.escape staleness-section cat/content in _build_staleness_section - #4119 add _escape_summary for injection-path summaries (Work/Personal/ Current Focus/Recent/Earlier/Background) - default-model silent no-op: factory injects host default chat model via a new host_llm slot (create_chat_model(name=None)); DeerMem prefers host_llm over build_llm(model). Zero-config extraction works out of the box again - MemoryConfigResponse: fix stale docstring (backend-agnostic shape; DeerMem knobs live under backend_config, not top-level - restoring flat would re-couple the API to DeerMem). Frontend audited: does not read /memory/config - _host_default_tracing_callback: restore langfuse assistant_id/environment - search: push category onto the ABC signature; DeerMem filters BEFORE the top_k slice (was filtered client-side after slicing -> starved results) - _do_update_memory_sync: split into wrapper+impl; bind trace_id into the request-trace ContextVar on the Timer/executor worker via a new trace_context_manager host hook (None trace_id left unbound - no fabrication) - client.py fact-CRUD now passes user_id (was writing to the global bucket while get_memory reads per-user) - _resolve_manager_class: fail-fast (raise ValueError) on an unresolved explicit manager_class instead of silently falling back to DeerMem (memory is persistent state - a wrong store is a silent data-integrity footgun) Upstream memory fixes dropped by the host->vendored rename conflict, re-ported to backends/deermem/deermem/core/ (+ deer_mem.py): - #4073 queue busy-timer-spin -> _reprocess_pending flag (core/queue.py) - #4074 null source.confidence in staleness -> _coerce_source_confidence (core/updater.py: _build_staleness_section + _apply_updates stale sort) - #4075 factsToRemove is optional (drop from _REQUIRED_MEMORY_UPDATE_TOP_LEVEL_KEYS) - #4076 null confidence in search ranking -> _coerce_source_confidence (deer_mem.py DeerMem.search) host_llm + trace_context_manager are host-injected via backend_config (factory in manager.py), keeping backends/deermem/ at exactly one `from deerflow` line (the ABC contract) - portability test preserved. Co-Authored-By: Claude <noreply@anthropic.com> * fix: resolve lint errors (F541 f-string without placeholders, E501 line too long) * fix(memory): restore hide_from_ui clarification preservation, expose mode Two memory-system fixes (F541/E501 lint was already fixed on this branch): - filter_messages_for_memory: restore default preservation of well-formed human_input_response clarification answers (v2 regression). The self-containment refactor made the bare function skip ALL hide_from_ui when no hook was passed, but upstream preserves well-formed clarification responses by default (test_hide_from_ui_human_input_response_is_preserved). Inline a host-agnostic _is_human_clarification_response mirror of read_human_input_response as the default keep-decision; the host-injected should_keep_hidden_message hook still overrides (production path unchanged). Portable package stays zero `from deerflow`. - /memory/config: expose `mode` (middleware|tool) in MemoryConfigResponse + the config/status endpoints + client.get_memory_config. mode is a host- shared, behavior-determining field missing from the response projection. Sync tests (mock .mode; e2e assert mode present). - Align manager_class field docstring with fail-fast behavior. Tests: filter/self-contained/portability (35) + memory-config (4) pass; ruff clean. Co-Authored-By: Claude <noreply@anthropic.com> * fix(memory): resolve ruff format failures in memory module + tests `make lint` runs `ruff format --check` in addition to `ruff check`; 8 memory files had pending format changes -- 7 pre-existing (deer_mem, updater, tools, test_memory_queue/router/search/tools) + message_processing from the hide_from_ui fix. Apply `ruff format`: whitespace/wrapping only, no logic change. 109 memory tests pass; ruff check + format --check both clean. Co-Authored-By: Claude <noreply@anthropic.com> * fix(memory): address PR review - legacy field migration, fact_id contract, path/docs Address willem-bd's review on PR head bc8bf0d4 (risk:high, persistent state): - config: auto-migrate pre-abstraction top-level memory.* DeerMem fields (storage_path, max_facts, debounce_seconds, model_name, token_counting, staleness_*, consolidation_*) into backend_config on load + warn, so an upgrade does NOT silently revert customized settings (was: silent extra='ignore' drop). model_name -> backend_config.model.model. Unknown top-level keys warned. - factory: resolve a relative backend_config.storage_path against runtime_home() (base_dir-relative, CWD-independent) to preserve pre-abstraction semantics; paths.py stays portable (no runtime_home import). - tools: memory_add uses the fact_id returned directly by create_fact instead of re-deriving it via content-key matching (coupled the tool to the backend's content normalization; could misreport a storage cap). create_fact now returns (memory_data, fact_id); gateway/client/tool updated. Fix terse {"error":"content"} -> {"error":"empty content"}. - app.py: update stale token_counting=="char" warm-up comment to point at manager.warm (DeerMem.warm re-checks char and returns early). - router: comment explaining reload_memory silent fallback vs fact 501 asymmetry (read-only degrade vs write fail-loud). - CHANGELOG: document breaking changes (/memory/config + client.get_memory_config shape flat->backend_config; custom storage_class path moved + __init__ must accept config) and the legacy-field auto-migration. - tests: add regression test pinning the per-user memory path ({storage_path}/users/{safe_user_id}/memory.json == host make_safe_user_id) across the abstraction; update create_fact mocks for (memory_data, fact_id). Tests: 273 passed (memory suite); ruff check + format clean. Co-Authored-By: Claude <noreply@anthropic.com> * fix(memory): address PR review - storage_path, max_facts, tracing, parsing Six review findings (willem-bd), each verified against upstream: - storage_path semantics (file -> root dir): migration drops file-style (.json) legacy values with a warning; factory raises if storage_path resolves to an existing file (avoid silent NotADirectoryError write failure). CHANGELOG + config.example.yaml comment updated. - create_memory_fact enforces max_facts again (via _trim_facts_to_max) and returns (memory, None) when the cap evicts the new fact; memory_add tool reports "not stored", client raises ValueError, POST /memory/facts -> 409. - max_facts trim uses _coerce_source_confidence (was raw f.get("confidence", 0) -> TypeError on non-float imported/legacy confidence, swallowed as silent update failure). - memory-tracing assistant_id restored to "memory_agent" (was "lead-agent" copy-paste; matches upstream + DeerMem run_name). - _is_human_clarification_response cross-checked against read_human_input_response (drift guard test). - empty-string legacy values skipped silently in migration (narrow fix, not broad "if not value" which would skip explicit bool False). 8 new regression tests. make lint + 406 memory tests pass. Co-Authored-By: Claude <noreply@anthropic.com> * fix(memory): address internal review - storage fail-fast, build_llm degrade, config warn, noop template Addresses 4 findings from the PR #4122 internal supplemental review (parallel to willem-bd's review, no overlap): - create_storage fail-fast: a misspelled/unimportable storage_class now raises ValueError instead of silently falling back to FileMemoryStorage. Memory is persistent state, so a wrong store is a data-integrity footgun; mirrors the existing manager_class resolution policy. (storage.py) - noop template create_fact signature: the commented template used keyword-only `content` and returned a bare dict, while DeerMem's actual create_fact takes positional `content` and returns tuple[dict, str|None] (the memory_add tool passes content positionally; gateway/client/tools all tuple-unpack). A backend copied from the template would 500 on fact-CRUD. Template fixed; delete_fact/update_fact templates left (callers compatible). (noop_manager.py) - build_llm graceful degrade: wrap init_chat_model in try/except, degrade to None + WARNING on failure (mirroring _host_default_llm) so a misconfigured explicit model does not crash app startup -- non-LLM memory ops still work and an update raises at runtime with the error logged. (llm.py) - from_backend_config unknown-key warning: log a WARNING for unknown backend_config keys (mirrors the host layer's load_memory_config_from_dict) so a typo like `storage_pat` does not silently fall back to the default and write memory to an unintended location. (config.py) Tests: rewrote 3 create_storage fallback tests to expect ValueError; added 4 tests (build_llm zero-config/degrade, from_backend_config warn/silent). make lint green; full memory suite passes. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: lllyfff <2281215061@qq.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: lllyfff <122260771+lllyfff@users.noreply.github.com> |
||
|
hataa
|
1300c6d36b
|
feat(authz): add pluggable AuthorizationProvider protocol and config scaffolding (Phase 0, #4063) (#4127)
Phase 0 of the RFC in #4063: scaffolding only, zero behavior change. New deerflow/authz/ package (sibling to deerflow/guardrails/): - AuthorizationProvider Protocol: authorize() + aauthorize() + filter_resources() - Principal/AuthzRequest/AuthzDecision/AuthzReason dataclasses - GuardrailAuthorizationAdapter: bridges AuthorizationProvider → GuardrailProvider so existing GuardrailMiddleware can enforce authz decisions without a new middleware New authorization config section (default enabled: false): - AuthorizationConfig wired into AppConfig alongside guardrails - Singleton load/reset mirrors GuardrailsConfig pattern - config.example.yaml documents the RBAC provider schema 29 tests covering protocol conformance, dataclass construction, adapter request/decision mapping, and config singleton behavior. Per RFC #4063 Phase 0 (foundations). Layer 1/2 wiring and Principal builder in services.py deferred to Phase 1. |
||
|
Mohammed Ansari
|
4af6178358
|
feat(trace): add agent observability with Monocle (#4024)
* Add Monocle tracing Enable Monocle (OpenTelemetry tracing for LLM apps) with one setup call plus the monocle_apptrace dependency. setup_monocle_telemetry auto-instruments the frameworks already in use and writes traces to .monocle/. Additive; no changes to application logic. * Config-gate Monocle telemetry in the Gateway lifespan Addresses review on #4024: moves setup_monocle_telemetry out of agents/__init__ import time into the Gateway lifespan, gated by MonocleTracingConfig (MONOCLE_TRACING env, default off). Warns on the Langfuse/global-OTel-provider conflict and relies on monocle_apptrace's own duplicate-setup guard and existing-provider attach. Pins monocle_apptrace>=0.8.8 (+ uv.lock), adds .monocle/ to .gitignore, adds tests (default-off / toggle-on / no import-time setup), and documents exporters, Okahu, and the VS Code viewer in README, config.example.yaml, and backend/AGENTS.md. * Clarify Monocle/Langfuse single-provider guidance Make the docstring, warning, and AGENTS.md consistent with the README: only one library can own the global OpenTelemetry provider; Monocle initializes at startup before Langfuse's per-run handler, so enabling both drops Langfuse's spans — enable one OTel tracer (LangSmith, a callback, coexists fine). * Address review: optional extra, exporter validation, off-box warning, tests Responds to the second review round. - Make monocle_apptrace an optional extra (deerflow-harness[monocle], re-exposed as deer-flow[monocle]) following the boxlite/tui precedent, so a default install no longer pulls the OpenTelemetry stack. It stays pinned in the dev group for the tracing tests, and enabling MONOCLE_TRACING without the extra raises a clear install error. - Warn loudly at startup whenever any exporter other than `file` is configured, since those move prompts, tool inputs/outputs, and completions beyond the local .monocle/ directory. - Validate MONOCLE_EXPORTERS against the known exporter names and require OKAHU_API_KEY when okahu is selected, mirroring the Langfuse pattern. Validation runs from Monocle's own init (not validate_enabled) so a config typo can never fail agent runs; errors surface at Gateway startup instead. - Grow the tests from 5 to 13: caplog coverage for the Langfuse-conflict and off-box warnings, exporter validation cases, a stronger import-time regression that asserts the global TracerProvider is not replaced, and a subprocess double-invoke test exercising the real check_duplicate_setup. - Docs: config.example.yaml block retitled to a dedicated tracing header; README documents the [monocle] install and scopes tracing to Gateway runs. * docs: align Monocle README section with the other tracing providers Lead with what Monocle is and captures, drop the install step (the dev group already ships monocle_apptrace via uv sync; unusual installs get the RuntimeError), and point the missing-package error at the repo-native command (uv sync --extra monocle / deerflow-harness[monocle]). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Address review: verified Langfuse coexistence, lifespan test, scope docs Responds to the third review round. The Langfuse conflict claim was wrong, verified empirically against langfuse 4.5.1 in both init orders: whichever library initializes second reuses the existing global TracerProvider and attaches its own span processor, so neither side loses spans. Dropped the warning and its tests, corrected the README, AGENTS.md, and config.example.yaml statements, and pinned the verified behavior with test_coexists_with_langfuse (real monocle + real langfuse in a subprocess, no mocks). One honest caveat documented: both processors see all spans, so Monocle's exporters also capture Langfuse's spans when both are enabled. Also from the review: - Document the Gateway-only scope in AGENTS.md: the lifespan is the sole call site, so the embedded DeerFlowClient and TUI are not instrumented; embedded users call setup_monocle_tracing_if_enabled() themselves. - Add test_gateway_lifespan_initializes_monocle pinning the lifespan wiring. - Comment why MonocleTracingConfig.is_configured is intentionally coarser than LangSmith/Langfuse (composite validation lives in validate() at startup). - Note that monocle_exporters_list takes the comma-separated string as-is. - Module-level importorskip("monocle_apptrace") so minimal installs collect the test module cleanly. * docs: reword Monocle intro sentence * fix(tests): run the import-time regression in a subprocess test_no_import_time_setup deleted deerflow.agents* from sys.modules and re-imported to force __init__ to re-execute. The re-import creates new module objects, and restoring the old sys.modules entries afterwards leaves the parent package's attribute bindings pointing at the new ones, so any later test that resolves a deerflow.agents.* dotted path (monkeypatch.setattr in test_summarization_middleware, test_thread_data_middleware, and others) failed with "module 'deerflow.agents' has no attribute ...". Run the check in a subprocess instead: the import is genuinely fresh, the assertion is stronger (the provider must still be the SDK-less proxy, proving nothing was installed at any point), and no module identity leaks into the rest of the suite. * Address review: console warning scope, embedded hint, honest naming, doc alignment Responds to the post-approval review round: - Scope the off-box exporter warning to the remote exporters (okahu, s3, blob, gcs): console writes to local stdout and no longer trips it. config.example.yaml's data-handling note now distinguishes file / console / remote likewise. - Rename MonocleTracingConfig.is_configured to is_enabled so the boolean reads as what it checks; the exporter-dependent credential check stays in validate(), run at Gateway startup. - Hint on the embedded path: build_tracing_callbacks() logs a debug line when MONOCLE_TRACING is set but setup never ran in this process, so embedded DeerFlowClient/TUI users are not left with silent no-op tracing. Backed by a process-global setup flag. - Re-export setup_monocle_tracing_if_enabled from deerflow.tracing, matching the package convention. - Note the deliberate fail-open-at-startup contrast with LangSmith/Langfuse in the lifespan, and the OTel SDK-internals dependency in the coexistence test. - Test hygiene: clear MONOCLE_* env in the tracing config/factory fixtures; reset the setup flag in the monocle test fixture; reword the README Langfuse-spans claim as the shared-provider inference it is. - Document that .monocle/ trace files are never rotated or cleaned up. * fix(tests): pin the factory logger level in the embedded-hint tests configure_logging() from earlier tests in the full suite pins an explicit INFO level on the logger hierarchy, so a root-level caplog.at_level(DEBUG) never sees the factory's debug hint. Scope caplog to deerflow.tracing.factory so the test is independent of suite ordering. * Address review: co-export disclosure, lifespan failure test, exporter parse dedup - Off-box warning now notes that Langfuse's spans are exported too when both providers are enabled and share the global OTel provider; pinned both ways by tests. - Pin the lifespan fail-open contract: a raising Monocle setup is logged and the Gateway keeps serving (pragma dropped now that the path is exercised). README notes a config error is reported at startup and tracing stays off until restart. - Hoist exporter parsing into MonocleTracingConfig.exporter_list so validate() and the off-box warning cannot diverge, and note the upstream coupling on the exporter allow-list. - Reduce config.example.yaml's Monocle block to a pointer; the capture, retention, and data-handling detail lives in README's Monocle section. --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
OrbisAI Security
|
8cc4b3abeb
|
fix(sandbox): the sandbox provisioner api exposes endpoints need to check API KEY (#4116)
* fix: V-001 security vulnerability Automated security fix generated by OrbisAI Security * fix(sandbox): wire provisioner API key through backend client and config RemoteSandboxBackend now accepts an api_key parameter and sends it as X-API-Key on all five provisioner HTTP calls (list, create, destroy, is_alive, discover). AioSandboxProvider reads provisioner_api_key from SandboxConfig and forwards it at construction time. SandboxConfig formally declares the field; config.example.yaml documents it under Option 4; docker-compose-dev.yaml threads PROVISIONER_API_KEY into both the provisioner and gateway containers so a single .env entry covers both sides. Tests: monkeypatch PROVISIONER_API_KEY and send X-API-Key headers in the five parametrized threading tests (previously 401-failing); new test_auth_middleware asserts /health is open, /api/* rejects no-header and wrong-key with 401, and accepts the correct key. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(provisioner): correct fail-closed auth docs and fix test mock signatures The first round of PR #4116 fixes left four blocking issues: - Mock signatures in test_remote_sandbox_backend.py (17) and test_aio_sandbox_provider.py (2) didn't accept the new headers= kwarg, causing TypeError on every provisioner HTTP call test. - sandbox_config.py and config.example.yaml described the auth as optional ("leave unset to disable") but the middleware is fail-closed: an unset PROVISIONER_API_KEY causes 401 on every /api/* request. - .env.example had no PROVISIONER_API_KEY entry, leaving users with an empty value and silent 401s. - No test covered the PROVISIONER_API_KEY="" fail-closed path. Fix all four: add headers=None to all mock signatures, correct the field description and example comment to state that both sides must have the same key set, add PROVISIONER_API_KEY to .env.example with generation guidance, add test_auth_middleware_unset_key, and add a logger.warning on auth rejection for observability. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> |
||
|
qin-chenghan
|
a94ea9325b
|
fix(agents): load SOUL.md from agent dirs without config.yaml (#4136)
* fix(agents): load SOUL.md from agent dirs without config.yaml (#4135) resolve_agent_dir requires config.yaml to be present in an agent directory (added in #3481 to fix #3390, where memory-only directories were mistaken for agent directories). However, SOUL.md loading does not depend on config.yaml. When an agent is configured externally (e.g. via DEER_FLOW_CONFIG_PATH) and the agent directory contains SOUL.md but no config.yaml, resolve_agent_dir skips the directory and load_agent_soul returns None. Add a fallback in load_agent_soul: if the resolved directory does not contain SOUL.md, check the per-user and legacy directories directly for SOUL.md. This preserves the #3390 fix for resolve_agent_dir (which is also used by load_agent_config) while allowing SOUL.md to load independently of config.yaml. * fix(agents): gate SOUL.md fallback on missing config.yaml per review Address review feedback on PR #4136: 1. Gate the fallback condition on not (agent_dir / config.yaml).exists() so it only fires when resolve_agent_dir returned its default path (no agent dir qualified), not when a properly-resolved per-user agent simply lacks SOUL.md. This preserves the per-user shadowing invariant. 2. Fix test_loads_soul_from_user_dir_without_config_yaml to actually exercise the fallback path: per-user memory-only dir (no config.yaml, no SOUL.md) + legacy dir with SOUL.md (no config.yaml) -> fallback finds legacy SOUL.md. 3. Add test_soul_not_leaked_from_legacy_when_per_user_has_config to verify the gate prevents legacy SOUL.md leaking into a per-user agent. 4. Patch get_effective_user_id in test_loads_soul_without_config_yaml for consistency and resilience. |
||
|
DanielWalnut
|
4e209827f3
|
feat(agent): Add subagent total delegation cap (#4115)
* fix subagent total delegation cap * fix embedded subagent run cap context * fix subagent cap config consistency * fix resumed subagent run cap boundary * fix legacy resume subagent boundary * address subagent cap review feedback --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Daoyuan Li
|
74392e1470
|
Fix require_mention gating on whitespace-only bot_login/mention_login (#4055)
github.bot_login and trigger.mention_login were read raw in the require_mention precedence chain, so a whitespace-only value (e.g. " ") never fell through to the working fallback the chain documents - Python truthiness lets " " win an `or` chain over agent.name or the operator default. The third link (channels.github.default_mention_login) was already correctly normalized and pinned by test_operator_default_blank_string_treated_as_none; this closes the gap for the other two by normalizing GitHubTriggerConfig.mention_login and GitHubAgentConfig.bot_login once, at the config layer, via a field_validator mirroring the pattern already used elsewhere in the config package. |
||
|
heart-scalpel
|
3bc3af2530
|
fix(runs): close multi-worker ownership gaps in run atomicity (#3948) (#4003)
* feat(runs): cross-process run ownership with lease + reconciliation (#3948) Implements work items 2 and 3 of the multi-worker P0 plan (docs/multi_worker.md). Work item 1 (Postgres startup gate, #3960) already landed; this PR makes run creation race-safe across worker processes and lets Postgres deployments recover orphaned inflight runs from crashed workers without mis-marking live runs as orphans. Work item 2 — cross-process atomic create_or_reject - Alembic revision 0004_run_ownership adds runs.owner_worker_id, runs.lease_expires_at, idx_runs_lease, and a partial unique index uq_runs_thread_active (one pending/running run per thread). The index is declared on RunRow.__table_args__ with sqlite_where + postgresql_where (mirroring uq_channel_connection_active_identity) so the empty-DB bootstrap path — which runs Base.metadata.create_all + alembic stamp head without executing any revision's upgrade() — also lands it on fresh deployments. Migration 0004 additionally creates it idempotently for legacy/versioned upgrades. - RunRepository.create_run_atomic is the new atomic primitive: - reject: INSERT directly; the partial unique index catches duplicate active runs; the manager surfaces the result as ConflictError. - interrupt/rollback: SELECT FOR UPDATE the conflicting rows, skip rows whose lease is still valid AND owned by another live worker (raise ConflictError — the INSERT would have failed on the index anyway, and a retry loop cannot make progress), cancel the rest in the same transaction, then INSERT the new row. Rows owned by this worker are interruptible regardless of lease state. - RunManager.create_or_reject dispatches to the store under the existing local lock; same-worker in-memory cancellation runs after the store commit succeeds. MemoryRunStore mirrors the same semantics for tests and database.backend=memory. Work item 3 — lease heartbeat + Postgres reconciliation - RunOwnershipConfig (lease_seconds=30, grace_seconds=10, heartbeat_enabled=false by default), registered as startup-only in reload_boundary.STARTUP_ONLY_FIELDS because the heartbeat background task is created once in langgraph_runtime() and is not rebuilt on config.yaml edits. - When heartbeat_enabled, each worker renewes leases on its own active runs with interval = lease_seconds / 3. The loop is bounded and stop-event-cancellable so shutdown is prompt. - reconcile_orphaned_inflight_runs now runs on every backend — the sqlite-only gate in app/gateway/deps.py is dropped in the same commit so there is no window where Postgres would mis-mark live Worker A runs as orphans. Reconciliation errors only runs whose lease is NULL (legacy pre-ownership rows) or older than grace_seconds. In single-worker mode (heartbeat off, NULL leases) all inflight rows reclaim immediately, preserving the pre-ownership recovery latency. - Heartbeat starts AFTER startup reconciliation and stops BEFORE the in-flight run drain on shutdown so the two cannot race. GATEWAY_WORKERS=1 with heartbeat_enabled=false keeps current behavior. Verified: 170 related tests + full backend suite (minus Docker-gated live tests) green; ruff check + ruff format clean. * fix(runs): tighten unique-violation handling and document clock-sync budget Three follow-up fixes to the cross-process run ownership work in #3948, surfacing during review. 1. _is_unique_violation: detect by driver-native signal, not message text The previous substring heuristic ("unique" + "violat", or "duplicate") missed SQLite's actual phrasing "UNIQUE constraint failed: <table>.<index>" — SQLite says "failed", not "violates", and never "duplicate". On SQLite the detector returned False, the reject path re-raised the raw IntegrityError, and clients saw HTTP 500 instead of ConflictError 409. The conversion is the load-bearing piece of the "store is source of truth" design but was untested — every atomic test used MemoryRunStore, which raises ConflictError directly and never reached this branch. Now prefers driver-native signals: psycopg pgcode/sqlcode "23505" and sqlite3 sqlite_errorcode SQLITE_CONSTRAINT_UNIQUE (reachable through SQLAlchemy IntegrityError.orig). Message matching stays as a fallback with SQLite's exact "unique constraint failed" phrase added. 2. interrupt/rollback: convert exhausted-retry IntegrityError to ConflictError The reject branch converts unique violations to ConflictError. The interrupt/rollback retry loop did not — on the 3rd attempt it re-raised the raw IntegrityError, leaking HTTP 500 for the same race condition that reject surfaces as 409. Symmetric conversion added after the loop; callers now see a consistent ConflictError regardless of strategy. 3. Document clock-sync requirement for multi-worker lease reconciliation reconcile_orphaned_inflight_runs compares another worker's UTC lease_expires_at against this worker's datetime.now(UTC). The only skew budget is grace_seconds (default 10s) — worst case, with the owning worker's heartbeat just about to fire, a peer whose clock is more than ~grace_seconds ahead can mis-reclaim a still-live run as an orphan. Documented in RunOwnershipConfig's docstring (with the math) and in config.example.yaml (with operational guidance), so operators in NTP-poor environments know to raise grace_seconds. Default unchanged: 10s is reasonable for NTP-synced K8s/cloud, and bumping it would slow recovery of genuinely dead workers (lease_seconds + grace_seconds from last heartbeat to reclaim). Tests: - test_create_run_atomic_reject_propagates_conflict_on_unique_violation: end-to-end against a real SQLite-backed RunRepository, pre-inserts an active run, asserts reject-strategy create surfaces as ConflictError rather than raw IntegrityError. - test_is_unique_violation_detects_real_sqlite_integrity_error: unit test for the detector against a real SQLite-raised IntegrityError; asserts driver-level sqlite_errorcode is SQLITE_CONSTRAINT_UNIQUE. - test_interrupt_exhausted_retries_surface_as_conflict_error: pins the symmetric 409 behavior after the retry loop exhausts. Verified: ruff check + ruff format clean; multi-worker + run_repository + owner_isolation + reload_boundary suites green. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(runs): close multi-worker ownership gaps in lease heartbeat and unique-violation detection Five code-review fixes from docs/multi_worker.md: 1. Drop unused ``claim_inflight_runs`` primitive — no caller anywhere. ``create_run_atomic`` does its own inline claim (SELECT FOR UPDATE + cancel) inside the INSERT transaction; a separate claim primitive would split that into two transactions and open a claim→INSERT race. Removes ~40 lines across base.py / memory.py / sql.py plus the unused ``now_iso`` parameter, freeing future RunStore implementations from providing it. 2. Broaden ``_renew_leases`` filter to renew pending/running runs owned by this worker even when ``record.task is None``. The previous ``task is not None`` requirement skipped the brief window between ``create_run_atomic`` inserting the row and the worker spawning the agent task; under event-loop load that window can approach ``lease_seconds``, after which peer reconciliation marks the run ``error`` (visible) or a peer's ``create_or_reject("interrupt")`` silently kills the queued run. Filter now: ``task is None or not task.done()``. 3. Document the unsynchronised ``record.lease_expires_at = new_expiry`` write. ``lease_expires_at`` is the only field on an existing record this path mutates; ``set_status`` / ``_persist_status`` touch other fields, so there is no concurrent writer to race against. Re-acquiring ``self._lock`` would serialise unrelated run mutations for no gain. 4. Gate ``_is_unique_violation`` message fallbacks on ``isinstance(current, (SAIntegrityError, sqlite3.IntegrityError))``. The driver-code path (pgcode/sqlite_errorcode) remains load-bearing; substring fallbacks are now belt-and-suspenders only for cases where the driver attribute isn't reachable through the cause chain. Without the gate, any application exception whose ``str()`` happens to contain "duplicate key" / "unique" + "violat" (CHECK constraint, validation error) would silently surface as HTTP 409 instead of 500. 5. Route ``update_lease`` through ``_call_store_with_retry`` for consistency with every other store call, and wrap ``await self._renew_leases()`` in ``_heartbeat_loop`` with ``except Exception: logger.warning(...)``. Previously a transient error from the snapshot path or an unexpected exception would kill the heartbeat task silently — after which no lease is ever renewed again and every active run eventually looks orphaned. ``except Exception`` lets ``CancelledError`` (BaseException since 3.8) propagate so shutdown cancellation still works. Regression tests: - ``test_heartbeat_renews_pending_run_before_task_is_spawned`` - ``test_is_unique_violation_does_not_misclassify_application_exception`` Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(runs): harden multi-worker migration, memory atomicity, and tz-naive lease comparison Three follow-up fixes to the multi-worker run ownership work: - migration 0004 dedupe pass: cancel superseded duplicate active rows per thread before creating the partial UNIQUE index ``uq_runs_thread_active`` so dirty DBs (Postgres deployments that had reconciliation skipped by the old sqlite-only gate, or any env that ran GATEWAY_WORKERS>1 before this PR) do not abort the alembic upgrade and block gateway startup. Keeps the newest active row per thread, marks the rest as error with an explanatory message. - MemoryRunStore.create_run_atomic interrupt/rollback path: split the single- pass loop into two passes (collect candidates, validate, then mutate) so a ConflictError raised on a later candidate does not leave earlier candidates half-interrupted. Mirrors the SQL store's transactional rollback semantics; the entire test_multi_worker_run_ownership.py suite runs against memory so this divergence was giving false confidence. - RunRepository.create_run_atomic interrupt path: coerce tz-naive ``row.lease_expires_at`` to UTC before comparing against the aware ``cutoff``. SQLite drops tzinfo on read despite ``DateTime(timezone=True)`` (this file's own comment acknowledges it), so the Python-side comparison raised ``TypeError: can't compare offset-naive and offset-aware datetimes`` whenever heartbeat was enabled on SQLite and a lease was non-NULL. Defaults (heartbeat off -> leases always NULL) masked it, but there was no guard against the combination. Follows the existing "naive is UTC" convention from ``coerce_iso``. Each fix ships with a regression test pinning the behavior. Co-Authored-By: heart-scalpel <heart-scalpel@users.noreply.github.com> Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(runs): enforce heartbeat for multi-worker, fix memory-store datetime comparison, lazy-import ConflictError in store layer Three fixes from code review: 1. Extend the startup gate (GATEWAY_WORKERS>1) to also require run_ownership.heartbeat_enabled=true. Without heartbeat every run has a NULL lease, so reconciliation treats all inflight rows as orphans and Worker B would kill Worker A's live runs on every rolling update or scale-up. 2. Fix MemoryRunStore.list_inflight_with_expired_lease to parse created_at as datetime instead of ISO string lexical comparison, and handle tz-naive lease values uniformly with the SQL store. 3. Store layer (sql.py, memory.py) now lazy-imports ConflictError inside create_run_atomic instead of importing from the higher RunManager layer at module level. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(runs): add owner check to update_lease, document create() assumption, restore deleted comment - update_lease (SQL + memory) now requires owner_worker_id match in WHERE clause so the primitive is safe by construction against misuse - create() docstring notes it bypasses atomic create_run_atomic and assumes no active run exists for the thread - restore explanatory comment in MemoryRunStore.aggregate_tokens_by_thread that was dropped in an earlier commit Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(runs): add psycopg3 sqlstate detection and periodic orphan reconciliation - _is_unique_violation now checks sqlstate attribute (psycopg3 uses this instead of pgcode). On Postgres, the only supported multi-worker backend, detection was falling through to the message-substring fallback. - _heartbeat_loop now runs reconcile_orphaned_inflight_runs every 3rd cycle (every lease_seconds) to catch orphans whose lease expires between pod restarts. Single-worker deployments are unaffected (heartbeat off). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: heart-scalpel <heart-scalpel@users.noreply.github.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Vanzeren
|
c2002d9fac
|
feat(memory): add memory tool sets (#4023)
* feat: add memory-as-tool mode alongside existing middleware mode - Add memory.mode config field (middleware|tool, default middleware) - Add search_memory_facts() for case-insensitive fact lookup - Add 4 memory tools: memory_search, memory_add, memory_update, memory_delete - Wire mode gating in factory.py and lead_agent/agent.py - 256 memory tests passing, zero regressions * fix: harden tool-mode memory scoping and docs * fix: address memory tool mode review feedback * fix(memory): address tool mode review feedback * fix: update config_version to 22 in values.yaml |
||
|
Tianye Song
|
9097642658
|
feat(memory): add memory consolidation to synthesize fragmented facts (#3996)
* feat(memory): add memory consolidation to synthesize fragmented facts When a fact category accumulates many individual entries, the LLM reviews them during the normal memory-update call (same invocation, no extra API cost) and decides whether groups of related facts can be synthesized into a single richer fact. This completes the memory lifecycle: extraction → guaranteed injection → staleness review → consolidation. - Select fragmented categories by min-facts threshold, surface the most fragmented groups first; prompt-layer caps aligned with apply-layer guardrails so the LLM never sees groups it cannot act on - Cap consolidated confidence at source maximum to prevent inflation; reject results below fact_confidence_threshold - Double-consume protection prevents a fact from being merged into multiple consolidation targets - Feature-gated at both prompt and apply time with per-cycle safety caps - Add 26 tests covering candidate selection, normalization, apply guardrails, and prompt integration * fix(memory): address consolidation correctness issues from PR review Six fixes based on maintainer review of #3996: 1. Deduplicate sourceIds in normalization — ["f1","f1"] previously bypassed the ≥2-distinct-sources check; dict.fromkeys collapses it to ["f1"] which is correctly rejected. 2. Run consolidation after max_facts trim — previously, sources were deleted then the merged fact could be evicted by the trim, leaving no record of either. Moving consolidation last ensures source facts exist in the post-trim index before removal. 3. Fix count= attribute in consolidation prompt — advertised the full category size but listed only max_sources IDs; now uses min(len(group), max_sources) to match what the LLM can act on. 4. Exempt staleness_protected_categories from consolidation candidates — mirrors the existing staleness-review contract so correction facts are never surfaced for merging. 5. Strip and default category in consolidation normalization — " " or " preference " are now normalised, matching _normalize_memory_update_fact. 6. Propagate sourceError from source facts into consolidated fact — correction context is no longer silently lost on merge. * fix(memory): add apply-time guardrails and tests for consolidation P1: mirror the staleness-pass defense-in-depth pattern — build allowed_source_ids from _select_consolidation_candidates at apply time so a protected-category or below-threshold fact proposed by the LLM is rejected regardless of model behavior. P2a: test that LLM-returned confidence is capped at max source confidence and that a capped result below fact_confidence_threshold is rejected. P2b: test that factsToConsolidate with consolidation_enabled=False is a no-op at apply time (35 tests, all pass). * fix(memory): address three correctness issues from second review round 1. Default consolidation_enabled=False — consolidation is lossy (source content is permanently replaced, only consolidatedFrom IDs preserved); new lossy features default to off. config.example.yaml updated to match. 2. Unify confidence coercion between prompt and apply — _build_consolidation_section now calls _coerce_source_confidence(fact) instead of an inline 0.0-default coercion, so a null-confidence fact renders with 0.50 in the LLM prompt and is capped at 0.50 at apply time (same value, same function). 3. Preserve staleness clock on merge — consolidated fact now carries the newest source's createdAt (not now) so aged information does not gain a fresh staleness-review window just by being consolidated; consolidatedAt is added as an explicit audit field. Three regression tests added (default=false, null-confidence consistency, createdAt policy); all guardrail tests now set consolidation_enabled=True explicitly so they test the guardrail, not the feature flag. 38 tests pass. * fix(memory): harden createdAt comparison and confidence handling 1. createdAt max via _parse_fact_datetime — replaces string max() which crashes on non-string createdAt (numeric unix timestamps) and sorts Z/+00:00 mixed formats incorrectly. Mirrors how staleness computes age. 2. Remove dead min(..., 1.0) — _coerce_source_confidence already clamps each source confidence to [0, 1], so max(source_confidences) ≤ 1.0 by contract; the outer min could never bind. 3. Clamp raw_llm_conf to [0, 1] before applying the source cap — out-of- range values like 1.5 are safe today (pinned by the cap) but defensively clamped first so the invariant holds even if the cap is ever loosened. 4. Doc: expand the apply-time guardrails comment to call out the protected- category exclusion via allowed_source_ids — this is the central safety property ("explicit user feedback is never silently merged away"). 5. Test: add test_confidence_fallback_to_max_source_when_llm_omits_field covering the else-branch (LLM omits confidence → uses max_source_conf). 6. Fix lint: reorder imports in test file (stdlib before third-party). 39 tests, all pass. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
hataa
|
266883b3dd
|
fix(subagents): inherit summarization middleware and harden step capture (#3875 Phase 3) (#4009)
Phase 3 of #3875 — subagents previously inherited none of the lead's context-compaction, so a deep-research subagent (max_turns up to 150) could accumulate >1M cumulative input before max_turns/timeout/token_budget engaged, even after Phase 2's budget capped the pathological tail. - Gate the subagent runtime chain on the SAME ``app_config.summarization.enabled`` switch the lead reads (per maintainer guidance in #3875), via the shared ``create_summarization_middleware`` factory. One config covers both chains; no separate ``subagents.summarization`` field. No-op when summarization is off (factory returns None). - ``skip_memory_flush=True`` on the subagent path: the factory otherwise attaches ``memory_flush_hook`` (when memory.enabled), which flushes pre-compaction messages into durable memory keyed by thread_id. Subagents share the parent's thread_id, so without skipping the hook a subagent's internal turns would pollute the PARENT thread's durable memory (#3875 Phase 3 review point). - Harden ``capture_new_step_messages`` to tolerate history contraction: summarization rewrites the messages channel via ``RemoveMessage(id=REMOVE_ALL_MESSAGES)``, shrinking len(messages) below the step-capture cursor. Without a reset, every step appended after the compaction point was dropped until length overtook the stale cursor (#3845 interaction, maintainer validation point (a)). Cursor now resets to the new tail; id/content dedup prevents re-emitting pre-compaction steps. - Couple the DEFAULT token-budget ceiling to ``summarization.enabled`` (#3875 Phase 3 review point): 1M when compaction is on, 2M when off (preserves Phase 2's deliberate headroom for summarization-off deep-research runs that can exceed 1M). A user-set budget (global or per-agent) always wins regardless of the switch. Flagged tunable. The summarization middleware does not implement ``consume_stop_reason``, so the Phase 2 guard-cap stop-reason channel is unaffected. Refs: https://github.com/bytedance/deer-flow/issues/3875 |
||
|
Ryker_Feng
|
ebc09ce130
|
feat(mcp): auto-promote deferred MCP tools from routing hints (#4019)
* feat(mcp): auto-promote deferred MCP tools from routing hints
When tool_search.enabled=true defers MCP tool schemas, PR1 routing hints
still require the model to spend a tool_search discovery round trip before
it can call the tool the routing metadata already points at. This adds a
McpRoutingMiddleware that matches the latest user message against PR1
routing keywords and promotes the matching deferred schemas before the
model call, removing that round trip.
Design (soft routing, opt-in, additive):
- Matches only the latest real HumanMessage (shared is_real_user_message
helper, reused by SkillActivationMiddleware so the two cannot drift);
case-insensitive substring match, no tokenizer dependency.
- Ordering: priority desc, then tool name asc; capped by the new global
tool_search.auto_promote_top_k (default 3, clamped 1..5). Does not add or
consume a per-tool auto_promote_top_k (PR1 schema unchanged); a per-tool
value is ignored with a DEBUG note.
- Returns a plain {"promoted": ...} state update (not a Command) and relies
on ThreadState.merge_promoted for union/dedupe, so auto-promote and a
model-triggered tool_search converge on the same catalog hash.
- Installed before DeferredToolFilterMiddleware on every deferred-tool path
(lead agent, subagent, embedded client, webhook via shared builders);
a construction-time assert rejects the reversed order. catalog_hash is
None / no routing index is a complete no-op, so bootstrap and ACP skip it.
- Privacy: never executes tools, never promotes policy-filtered tools, adds
no routing keywords or matched tool names to trace metadata or INFO/WARN
logs.
No behavior change when tool_search.enabled=false.
Tests: index construction, matching semantics, middleware state updates,
same-cycle deferred-filter interaction, lead/subagent/embedded-client
builder wiring + order invariant, config clamping, config.example.yaml
parseability, and privacy assertions.
* refactor(mcp): address auto-promote review nits
- executor: access app_config.tool_search.auto_promote_top_k directly to match
the lead-agent and embedded-client paths (drop the over-defensive getattr that
masked missing config); update the subagent test mock to carry tool_search.
- tool_search / mcp_routing_middleware: cross-reference the duplicated routing
priority/keyword normalization between the builder and the middleware's
defensive _normalize_index so they cannot silently drift.
- MCP_SERVER.md: document that auto-promote keyword matching is a case-insensitive
substring test (not word-boundary), advising distinctive keywords.
|
||
|
Vanzeren
|
dc38d2d003
|
perf(boxlite): add benchmark-driven warm-pool reclaim tuning (#4001)
* bench: add provider-agnostic sandbox benchmark with BoxLite warm pool results - scripts/bench_sandbox_provider.py: CLI for measuring acquire/run/release across providers, scenarios, workloads, and concurrency levels - scripts/summarize_bench.py: JSONL aggregation with p50/p95/p99 tables - bench_results.jsonl: 110 turns across 7 scenarios on real BoxLite 0.9.7 Key findings: cold acquire: ~860ms warm reclaim: ~14ms (60x speedup) release: ~0ms warm_hit_rate: 95% (warm_same_thread) * perf(boxlite): skip health check for recently-released warm pool boxes Boxes released within health_check_skip_seconds (default 5.0s) are promoted directly without the ~14ms echo-ok round-trip. A VM alive seconds ago is overwhelmingly likely to still be alive. Add sandbox.health_check_skip_seconds config option. Set to 0 to always health-check (old behaviour). Benchmark (warm_same_thread, noop, 20 iters): acquire p50: 14.9ms → 0.0ms total p50: 29.9ms → 14.0ms * chore: move benchmark scripts into backend/scripts/benchmark/ * fix: address BoxLite benchmark review findings * fix(boxlite): only skip warm reclaim checks for released boxes * fix(benchmark): keep BoxLite shim workaround off the event loop * fix(boxlite): invalidate dead boxes from command path * test(boxlite): cover skip window and invalidation edge cases * fix(boxlite): treat sandbox-has-been-closed as terminal in _exec * fix(boxlite): harden warm-pool reclaim and benchmark accounting * fix(boxlite): validate warm-pool reclaims by default * fix(config): expose boxlite health-check skip setting * fix(boxlite): tighten failure classification and benchmark workaround * Update config_version to 21 in values.yaml --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Ryker_Feng
|
5ba25b06ec
|
feat(mcp): add MCP routing hints (#4004)
* feat: add MCP routing hints * test: isolate mcp routing prompt config * fix: address mcp routing review feedback |
||
|
hataa
|
c9fb9768d4
|
fix(subagents): unify guardrail caps on additive stop_reason + add token_budget (#3875 Phase 2) (#3980)
Phase 2 of #3875. Two guardrail axes can end a subagent run early — the turn budget (GraphRecursionError) and the token budget (TokenBudgetMiddleware) — and both now surface *why* through one additive `subagent_stop_reason` field instead of a status enum. This completes and course-corrects Phase 1 (#3949), which shipped the turn-budget cap as a `max_turns_reached` status enum. The agreed Phase 2 design replaces that enum with an optional `stop_reason` field (token_capped | turn_capped | loop_capped): a new enum value would break v1 consumers, while an additive field is ignored by older frontends and ledger readers. `max_turns_reached` and SubagentStatus.MAX_TURNS_REACHED are removed. - subagents.token_budget config (default enabled, 2,000,000 tokens, warn 0.7) with per-agent override; TokenBudgetMiddleware is now attached in build_subagent_runtime_middlewares so the cost-ceiling backstop engages for every subagent. The hard-stop does not raise — it strips tool_calls and lets the run finish with a final answer, recording the cap on a per-run consume_stop_reason() accessor. - executor.py: on normal completion it reads consume_stop_reason() and stamps completed + token_capped when the budget fired; on GraphRecursionError it recovers the last AIMessage partial (completed + turn_capped) or, if nothing usable survived, failed + turn_capped. SubagentResult gains stop_reason. - status_contract.py / contracts/subagent_status_contract.json (v2) / frontend subtask-result.ts: additive subagent_stop_reason field, pinned by test_status_values_match_contract / test_stop_reason_values_match_contract. - task_tool.py + delegation_ledger.py: drop the max_turns_reached paths; the ledger captures stop_reason and renders model-facing "capped" guidance so the lead reuses a capped completion knowingly. The 2,000,000-token default is deliberately loose (tighten to taste) — it would have roughly halved the reported 4.4M burn while leaving legitimate deep-research runs (max_turns=150) room. Subagent summarization is a follow-up. |
||
|
Ryker_Feng
|
01dc067997
|
feat: add composer input polishing (#3986)
* feat: add composer input polishing * Revert "Merge branch 'main' into feat/input-polish" This reverts commit 5b6ceccf0db3092bc62fde3b05e7816829601756, reversing changes made to 45fbc57fef5fa5fd878cf0176c37f3e3bc7ebef6. * Merge main into feat/input-polish * style(frontend): format input helper polish guard * fix(input-polish): address composer polish review findings Frontend - Add a cancel affordance to the in-flight polish status pill that calls abortInputPolishRequest(), so a slow/hung provider no longer hard-locks the composer for up to stream_chunk_timeout with a page reload (and draft loss) as the only escape. - Reset promptHistoryIndexRef/promptHistoryDraftRef when a rewrite is applied (and on undo), so a stale history-browse index can no longer let the next ArrowDown silently overwrite the polished draft. - Disable polishing while an open human-input card is present, matching the frontend/AGENTS.md rule that composer entry points defer to the card so card-reply metadata is preserved. - canPolishInput now reuses parseGoalCommand/parseCompactCommand instead of a third hardcoded reserved-command regex, and drops the phantom /help entry (no /help parser exists in the composer), so future builtins only need to be taught to the existing parsers. Backend - Extract the non-graph one-shot LLM path (build model + inject Langfuse metadata + system/user invoke + text extract) into deerflow.utils.oneshot_llm.run_oneshot_llm, shared by the input-polish and suggestions routers so tracing-metadata and invocation shape cannot drift between the two copies. - strip_think_blocks gains truncate_unclosed (default True, preserving the suggestions/goal JSON-prep behavior); input polish passes False so a draft that legitimately contains a literal <think> substring is no longer truncated into a partial rewrite or a spurious 503. - Validate the empty-check and max_chars boundary against the same stripped view of the draft that is sent to the model, so the user-facing length boundary and the model input can no longer disagree. Tests / docs - Backend: literal-<think> preservation, whitespace-only rejection, and normalized-length/model-input agreement cases; suggestions tests repoint the create_chat_model patch to the shared helper module. - Frontend: helper unit tests updated for the /help/reserved-command change; a new Playwright case covers cancelling an in-flight polish request. - backend/AGENTS.md documents the shared one-shot helper and the polish normalization/think-tag behavior. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
AochenShen99
|
658c39ccf7
|
feat(skills): Add native SkillScan phase 1 for skills (#3033)
* Add phase 1 skill static scanning * Rework SkillScan phase 1 as native scanner * refactor(skillscan): align phase 1 with trimmed RFC contract - SecurityFinding: 7 fields (rule_id, severity, file, line, message, remediation, evidence); category/analyzer derive from the rule_id prefix, confidence/column/fingerprint/metadata removed - scan_archive_preflight()/scan_skill_dir() are pure functions: no ScanContext, no policy schema; CRITICAL-blocks is a code constant and skill_scan.enabled is applied by enforce_static_scan()/callers - secret-* evidence is redacted before findings leave the scanner - de-dup keys on (rule_id, file, line) so repeated occurrences keep distinct locations for agent self-correction - cloud-metadata detection consolidated into network-cloud-metadata - nested zip members get a one-level stdlib magic-byte peek; an executable member escalates package-nested-archive to CRITICAL - install metadata sidecar removed (Phase 7 decides if it is needed) - rule specs moved next to their analyzers; skillscan/rules/ removed - tests updated + new anchors: redaction, dedup lines, nested-zip escalation, single cloud-metadata rule, bundled-skill zero-CRITICAL * fix(skillscan): tighten reverse-shell/secret/archive scan rules from review Address PR #3033 review feedback on the native SkillScan analyzers: - Reverse-shell false positives: split shell detection by signal strength (/dev/tcp/, nc -e stay CRITICAL; bash -i, mkfifo -> new HIGH shell-reverse-shell-heuristic, warn->LLM). The Python check is now AST-anchored on real socket.socket/os.dup2/subprocess call sites instead of raw-text substring matching, so prose/docstrings no longer hard-block. - Secret evidence: _redact_secret_evidence returns [redacted] with no secret bytes (was value[:6], which leaked 2 real token bytes past the prefix). - Archive DoS: cap outer archive member count (_MAX_ARCHIVE_MEMBERS=4096); scan_archive_preflight early-aborts with a package-too-many-members CRITICAL finding (routes through the existing blocked->400 fail-closed path). - shell-destructive-command: broaden the rm -rf matcher to sensitive system roots (/home, /usr, /*, --no-preserve-root /) while leaving safe subpaths unflagged. - Dead code: collapse _decode_text_for_analysis to a single decode path and drop the unused _TEXT_SUFFIXES set and _has_text_shebang helper. - local_skill_storage: document why the host_path branch keeps app_config possibly-None (lazy kill-switch resolution; avoids eager get_app_config in config-free environments such as CI). Tests: new negative/positive coverage in test_skillscan_native.py. Full backend suite 6616 passed, 26 skipped. |
||
|
He Wang
|
fd41fdb065
|
feat(middleware): add structured tool result meta and tool-progress state machine (#3601)
* feat(middleware): add structured tool result meta and tool-progress state machine
feat:
- Add tool_result_meta.py: ToolResultMeta dataclass (status/error_type/retryable/
recoverable_by_model/recommended_next_action/source) + normalize_tool_result and
stamp_exception_meta utilities; classifies every ToolMessage regardless of path
- Add ToolProgressMiddleware: per-(thread_id, tool_name) state machine ACTIVE →
WARNED (hint injected as HumanMessage) → BLOCKED (call short-circuited); Jaccard
near-duplicate detection for repeated successful results; auth/config/internal
errors bypass WARNED and go directly to BLOCKED; LRU-bounded thread state store
- Add ToolProgressConfig: all thresholds configurable (stagnation_threshold,
warn_escalation_count, jaccard_similarity_threshold, exempt_tools, etc.);
disabled by default (enabled: false)
- Wire ToolProgressMiddleware as outer wrapper around ToolErrorHandlingMiddleware
in _build_runtime_middlewares so it receives results already carrying
deerflow_tool_meta
fix:
- ToolErrorHandlingMiddleware now calls stamp_exception_meta on exception path and
normalize_tool_result on success path so every ToolMessage carries deerflow_tool_meta
test:
- Add test_tool_result_meta.py: 26 cases covering all classification paths,
stamp_exception_meta, and normalize_tool_result Command passthrough
- Add test_tool_progress_middleware.py: 27 cases including full async paths,
Jaccard duplicate detection, LRU eviction, hint injection, and malformed meta
passthrough
- Extend test_tool_error_handling_middleware.py: middleware ordering invariant and
meta stamping on exception
docs:
- Add tool_progress section to config.example.yaml with all fields and descriptions
- Update CLAUDE.md middleware chain documentation (entries 8-9)
Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
* fix(middleware): recoverable errors stay WARNED; fix auth keyword shadowing
fix:
- WARNED is terminal for recoverable_by_model=True errors (no_results, not_found,
permission); hint re-injected on each problem call instead of escalating to
BLOCKED, so the model can retry with different parameters (e.g. fresh query,
new URL) without being hard-blocked by a prior stagnation count.
Non-recoverable (rate_limited, transient) still escalate WARNED → BLOCKED
after warn_escalation_count more problems; auth/config/internal remain
immediately BLOCKED.
- Remove bare "api key" keyword from auth classification rule so "no api key
configured" correctly classifies as config (not auth), producing the accurate
block-reason text for the model.
docs:
- CLAUDE.md: document all three ToolProgressMiddleware transition paths
- config.example.yaml: update inline state-machine comment to match new paths
test:
- test_recoverable_errors_stay_warned_indefinitely: WARNED never escalates for
recoverable errors regardless of how many problem calls accumulate
- test_recoverable_error_re_injects_hint_past_escalation: hints continue past
the escalation zone for recoverable errors
- test_no_api_key_is_config_not_auth: regression guard for keyword shadowing fix
Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
* fix(tool_result_meta): add JSON error extraction and fix source classification
fix:
- Fix non-standard error path: source was "exception" but should be "tool_return"
- Add _extract_json_error_text to isolate JSON error fields from noisy JSON bodies
(e.g. Brave Search {"error": "...", "query": "..."} — query keywords no longer
pollute error classification)
- Add success-path JSON extraction to catch tools that return HTTP 200 with a JSON
error body (status="success" but {"error": "API key not configured"})
- Add _SEMANTIC_ZERO_ERROR_STRINGS frozenset to suppress false positives from tools
that use {"error": "none"} / {"error": "null"} / {"error": "ok"} as success signals
- Document that stamp_exception_meta always overwrites existing TOOL_META_KEY
(exception-derived classification is authoritative over tool return-time stamps)
test:
- Add parametrized regression tests for all semantic-zero error strings
- Add tests for non-standard error path source field
- Add tests for JSON error extraction (nonstd, success-path, numeric, falsy values)
- Correct test comment for test_no_api_key_is_config_not_auth
Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
* fix(tool_progress_middleware): fix 6 bugs, add terminal guard and structured logging
fix:
- H1: fix exempt_tools empty-set silently ignored — use `is not None` instead of
truthiness check so ToolProgressConfig(exempt_tools=set()) correctly disables all
exemptions
- Fix _get_block_reason creating phantom LRU entries via _get_state (write path);
now uses dict.get + explicit move_to_end on read path only
- Fix _pending memory leak: LRU eviction of _phase_states now synchronously removes
all (evicted_thread, *) keys from _pending
- Fix _assess_and_transition missing terminal guard for blocked state — a recoverable
error result could silently demote blocked → warned in concurrent-race scenarios;
early return preserves terminal semantics
- Fix recent_word_sets window: stored [-5:] but is_near_duplicate only compared [-3:];
align to [-3:] and change type list→tuple (prevents accidental in-place mutation
across dataclasses.replace shallow copies)
- Fix _format_hint missing "success" key and "continue" action: Jaccard near-duplicate
results produced the generic fallback instead of a specific actionable message
feat:
- Add structured state-transition logging (ACTIVE/WARNED/BLOCKED transitions, blocked
intercepts, hint injection debug log)
test:
- Add regression tests for all 6 bug fixes (H1, phantom LRU, pending leak, terminal
guard, window alignment, format_hint near-dup)
- Add Jaccard near-threshold boundary test (7/9 vs 8/9 Jaccard)
- Add production min_words=10 skip test for short content
- Add exempt_tools empty-set and None round-trip tests
- Add _augment_request deduplication test
- Add before_agent current-run preservation test
- Add structured logging tests (WARNED/BLOCKED/ACTIVE/intercepted/debug)
Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
* chore(config): remove unused backward-compat fields from ToolProgressConfig
Remove max_calls_per_intent and window_size fields that were marked
"Retained for backward compatibility; not used by the current state machine"
when the state machine was introduced. Pydantic v2 ignores unknown fields
by default, so existing config.yaml files with these keys remain valid.
Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
* fix(tool_progress): address PR review and multi-agent review findings
fix:
- Remove <80-char length gate for partial_success; only _PARTIAL_MARKERS now
- Add word-boundary regex for numeric HTTP codes (401/403/404/500) to avoid
false positives like "500ms" or "4010 rows" triggering hard-block
- Add "task" to default exempt_tools (delegation primitive, not a search tool)
- Remove move_to_end() from _get_block_reason read path; blocked threads were
permanently warm in LRU, starving active threads of eviction slots
- Add _reset_blocked_states in before_agent: scope BLOCKED and WARNED states
to a single run; clear recent_word_sets so stale Jaccard windows don't cause
false near-duplicate detections in the next run
- Compute word_set() lazily (only for success results); cap content at 8192
chars to bound memory and CPU cost on large tool results
- Remove unused retryable field from ToolResultMeta (no consumer existed)
- Add isinstance-based ordering guard and warning log for missing meta
- Fix JSON-without-error-key fallback: use _UNKNOWN_ERROR instead of
classifying incidental field values (e.g. {"user_id": 401} → auth → stop)
- Fix _extract_json_error_text: use json.dumps for dict/list error fields
instead of str() which produced Python repr matching config rules spuriously
- Add "no results found"/"no content found"/"no images found" to _PARTIAL_MARKERS
so success responses with empty results trigger stagnation detection
- Fix immediate-block path to increment consecutive_problems (was left at 0)
- Fix _queue_assessment: skip phantom _pending entries for evicted threads
- Bump config_version 13→16 (upstream added 14/15; tool_progress is additive)
test:
- Update test_short_content_is_partial → test_short_terse_success_is_not_partial
- Add parametrized test_numeric_keyword_word_boundary (8 positive + negative cases)
- Add test_before_agent_resets_blocked_states_for_new_run (strengthened assertions)
- Add test_before_agent_resets_warned_states_for_new_run
- Add test_missing_meta_on_non_exempt_tool_emits_warning
- Add test_middleware_ordering_guard_raises_when_progress_is_inner
- Add test_auth_error_immediately_blocked asserts consecutive_problems == 1
- Add tests for JSON-without-error-key, dict error field, no-results partial_success
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(tool_progress): address second PR review — perf, architecture doc, concurrency note
fix:
- Extract content.lower() once before _PARTIAL_MARKERS check in normalize_tool_message;
previously computed up to 7× per call inside the generator (once per marker)
docs:
- Add division-of-labor paragraph to ToolProgressMiddleware module docstring explaining
coexistence with LoopDetectionMiddleware: result-quality guard (per-tool BLOCK) vs
call-pattern guard (whole-turn hard-stop); no shared state, no double-stop risk
- Add threading.Lock comment explaining why asyncio.Lock is not used (short critical
sections, must also protect sync wrap_tool_call path from subagent executor threads)
- Update backend/CLAUDE.md entry 8 with division-of-labor summary; fix entry 9
(remove stale retryable field reference, add missing recoverable_by_model/source)
test:
- Add test_tool_progress_and_loop_detection_coexist_without_interfering: drives both
middlewares to WARNED state simultaneously, verifies independent state, independent
hint queues, and no cross-contamination; uses snapshot copy for final assertion
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(tool_progress): reset all tool states at run boundary; fix semantic-zero test validity
fix:
- _reset_run_states (formerly _reset_blocked_states) drops the phase filter and
resets all tracked (thread, tool) pairs unconditionally at before_agent; ACTIVE
tools with sub-threshold consecutive_problems or cached recent_word_sets no longer
bleed into the next run, preventing spurious WARNED transitions on clean R2 calls
- test_normalize_json_semantic_zero_error_string_not_treated_as_error: replace
{error_value!r} f-string (produces invalid JSON with single quotes) with
json.dumps so _extract_json_error_text actually parses the payload and the
_SEMANTIC_ZERO_ERROR_STRINGS guard is exercised, not bypassed at json.loads
test:
- add test_before_agent_resets_active_state_consecutive_problems_and_word_sets to
lock the ACTIVE-phase run-boundary reset: drives tool to active/cp=1/ws≠() in R1,
asserts both fields are zero/empty after before_agent fires for R2
* docs(tool_progress): document intentional per-run reset vs LoopDetection thread-scoped retention
Addresses reviewer observation in PR #3601 that _reset_run_states diverges
from LoopDetectionMiddleware's cross-run scoping policy without explanation.
Expands the _reset_run_states docstring to record the intentional design
choice: ToolProgressMiddleware resets per-run because result-quality errors
(rate_limited, transient) are time-bound and may resolve between turns —
retaining stale counters would risk false-positive BLOCKED calls.
LoopDetectionMiddleware retains history across runs because call-pattern
loops are time-invariant. The divergence is by design, not oversight.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(middleware): restore ReadBeforeWriteMiddleware as outermost write gate
A merge conflict resolution had accidentally placed ReadBeforeWriteMiddleware
after ToolErrorHandlingMiddleware (inner), reversing the original intent from
b81334cc where it was the outermost write gate before ToolErrorHandling.
fix:
- Restore ReadBeforeWriteMiddleware to outer position: ReadBeforeWrite →
ToolProgress → ToolErrorHandling. Blocked writes now return immediately
without consuming a ToolProgress slot.
- Add normalize_tool_result call on blocked ToolMessages so they carry
deerflow_tool_meta (recoverable_by_model=True) even though they bypass
ToolErrorHandlingMiddleware.
test:
- Add test_blocked_write_has_deerflow_tool_meta (sync + async) to lock the
normalize_tool_result behavior on blocked writes.
- Fix chain order assertions in TestChainWiring and
test_build_lead_runtime_middlewares_chain_order_matches_agents_md.
docs:
- Renumber AGENTS.md items: 10→ReadBeforeWrite, 11→ToolProgress,
12→ToolErrorHandling; update descriptions to reflect outermost-gate design.
- Fix stale cross-reference: LoopDetectionMiddleware (item 23) → (item 25).
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4 <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
|
||
|
Tianye Song
|
28f2b07b79
|
feat(memory): add staleness review to prune silently-outdated facts (#3860)
* feat(memory): add staleness review to prune silently-outdated facts
Facts created long ago may become outdated without any future conversation
explicitly contradicting them ("Silent Staleness"). This adds a staleness
review mechanism that surfaces aged facts to the LLM during the normal
memory-update call so it can semantically judge whether each is still valid.
- New MemoryConfig fields: staleness_review_enabled, staleness_age_days,
staleness_min_candidates, staleness_max_removals_per_cycle,
staleness_protected_categories
- New STALENESS_REVIEW_PROMPT section injected into MEMORY_UPDATE_PROMPT
when enough stale candidates exist
- New staleFactsToRemove output field in the LLM response schema
- Safety cap limits max removals per cycle, keeping lowest-confidence
entries when the LLM returns more than the cap
- Correction facts (category=correction) are protected by default
- Observability via structured logging of each removal with reason
- 32 unit tests covering parsing, selection, triggers, formatting,
normalization, safety cap, and integration
* fix(memory): add deterministic guardrail for staleness removals
_apply_updates previously removed any fact id the LLM returned in
staleFactsToRemove without verifying it was in the actual staleness
candidate set. An LLM slip could silently delete protected-category
facts (e.g. correction) or fresh facts, defeating the stated guarantee.
Now intersect stale_ids_to_remove with _select_stale_candidates before
the safety cap, making the protection independent of both model behavior
and the staleness_review_enabled flag.
Add three regression tests:
- test_protected_category_fact_refused_at_apply
- test_non_aged_fact_refused_at_apply
- test_guardrail_runs_when_staleness_review_disabled
* docs(memory): sync AGENTS.md staleness config + simplify datetime parsing
Address reviewer feedback from PR #3860:
- Add staleness workflow step and 5 new config fields to backend/AGENTS.md
- Simplify _parse_fact_datetime: drop manual Z→+00:00 replace, Python 3.12+ fromisoformat handles Z natively
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
|
||
|
Vanzeren
|
67c8ade375
|
feat(sandbox): warm pool for boxlite (#3951)
* feat: add boxlite SDK as harness dependency * test: add fake SimpleBox fixtures for BoxLite warm pool tests * feat: add deterministic sandbox_id and warm pool fields to BoxliteProvider * feat: pass deterministic sandbox_id to SimpleBox name * feat: warm pool lifecycle — park on release, reclaim on acquire - release(): parks VMs in _warm_pool with timestamp instead of closing - _reclaim_warm_pool(): health checks warm boxes via echo ok - acquire(): tries warm pool reclaim before creating new boxes - Deterministic sandbox_id ensures thread isolation * feat(boxlite): idle reaper, replica enforcement, warm-pool shutdown/reset - Task 6: idle reaper daemon thread destroys expired warm-pool boxes - Task 7: replica enforcement evicts oldest warm-pool box when at capacity - Task 8: shutdown() stops idle checker first, destroys all boxes (active+warm); reset() clears warm pool Tests: 17 passed (4 new: idle reaper, replica enforcement, shutdown, reset) * fix(boxlite): harden warm pool lifecycle races * docs(boxlite): document warm pool configuration * fix(sandbox): log warning when evicting oldest warm box is failed Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix(boxlite): stop provider lifecycle on reset * fix(boxlite): make runtime an optional dependency * test(boxlite): remove unused warm pool lookup * docs(boxlite): clarify optional runtime support * refactor: extract shared WarmPoolLifecycleMixin for sandbox warm-pool lifecycle Introduce deerflow.community.warm_pool_lifecycle.WarmPoolLifecycleMixin owning idle-checker loop, warm-pool expiry, oldest-warm eviction, replica counting, and soft-cap logging. Move AioSandboxProvider and BoxliteProvider onto the mixin; keep AIO active-idle cleanup local and delegate only warm-pool expiry to the shared helper. BoxliteProvider also gains: - Prefixed box names (deer-flow-boxlite-*) for startup orphan reconciliation - _reconcile_orphans() adopting surviving boxes from a prior process - Pinned timeout forwarding: command timeout now bounds both BoxLite SDK exec(timeout=...) and the loop bridge .result(timeout) - reset() reworked as lightweight registry clear (boxes -> warm pool, no close, no idle-reaper stop, no loop close) so reset_sandbox_provider() config switches are safe; shutdown() remains the teardown path Backward-compatible: AIO DEFAULT_IDLE_TIMEOUT/DEFAULT_REPLICAS/ IDLE_CHECK_INTERVAL stay importable; Boxlite IDLE_CHECK_INTERVAL stays monkeypatchable. --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> |