mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-08-06 21:08:43 +00:00
245 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Aari
|
70fb91654d
|
fix(gateway): seed branch run-events so inherited history survives forking (#4385)
* fix(gateway): seed branch run-events so inherited history survives (#4380) The thread feed (GET /messages, /messages/page) reads the run-event store, but branch creation only wrote checkpoint state - a fresh branch had no message rows, so the parent history vanished from the UI as soon as the branch's first run refreshed the feed. Seed the branch's run_events from the same checkpoint snapshot the branch was created from, mirroring RunJournal's message-event contract (event types, hidden-message rules, original-user-text restoration). Best-effort: a seeding failure degrades to the old behavior and is reported as history_seed_mode=failed. * docs(gateway): correct branch-seed docstring on RunJournal divergences The "consumers cannot tell a seeded row from a journaled one" claim was overstated for AI rows: seeded rows omit run-scoped enrichment (usage / latency_ms / llm_call_index) and stamp caller=lead_agent rather than the message's original caller, neither recoverable from a checkpoint message. Rewrite the docstring to state these divergences explicitly and note they are display-invisible today (no consumer indexes those keys; per-message caller drives no attribution). Also add a code comment marking the hide_from_ui filter as intentionally stricter than the live paths. * fix(gateway): seed dict-shaped checkpoint messages + persist hidden AI/tool rows Two review-driven fixes to build_branch_history_seed_events: 1. Checkpoint messages can arrive as model_dump()-shaped dicts (the branch-matching helpers in threads.py already handle both BaseMessage and dict). The seed only handled BaseMessage, so a dict-backed checkpoint seeded nothing and the branch reported skipped_empty while history existed. Coerce dicts back to BaseMessage via messages_from_dict (faithful: tool_calls / tool_call_id / additional_kwargs survive); unparseable dicts are dropped best-effort. 2. RunJournal.on_llm_end and _persist_tool_result_message persist hide_from_ui AI/tool rows unconditionally (the frontend hides them client-side); the hide check only gates the reconciliation pass. The seed dropped them, so a hidden turn vanished from a forked feed and seeded rows diverged from journaled ones. Match RunJournal and write them, restoring true row-level parity. Adds tests for dict deserialization, the unparseable-dict drop, and the hidden AI/tool persistence contract. |
||
|
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. |
||
|
lllyfff
|
01a89f2379
|
[feat] memory: pluggable MemoryManager interface for backend onboarding (#4326)
* refactor(memory): pluggable MemoryManager interface for backend onboarding
Optimize the MemoryManager interface layer so new backends (mem0/openviking)
onboard with less code and the contract stays stable as capabilities are
added. A minimal backend now implements only from_config + add + get_context
(verified by test_memory_manager_interface.py::_MinimalBackend onboarding via
the factory); the factory no longer knows a backend's private hooks.
- MemoryManager: ABC -> pydantic BaseModel; three-tier methods (tier-1
add/get_context abstract; tier-2 management defaults; tier-3 optional hooks
warm/reload/fact + on_pre_compress/on_turn_start). Dropped 3 self-serving
hooks. 6 hasattr probe sites -> direct call + try/except NotImplementedError.
- from_config classmethod: factory thins to resolve + inject storage_path +
collect host hooks + call from_config; DeerMem-specific hook consumption
moved from factory to DeerMem.from_config.
- Invariants: @model_validator (mode='tool' requires search via supports_search
ClassVar); DeerMemConfig storage_path-is-file check moved here from factory.
- Async: aadd/aget_context/asearch default to the sync path (speculative).
- Callbacks: MemoryCallbacks + LangfuseMemoryCallbacks; on_memory_llm_call
subsumes tracing_callback (same signature/timing/mutation); deleted the
tracing_callback field. DeerMem decoupled from langfuse (portability).
- noop keeps read-op empty overrides (avoids router 500s on the
disable-memory-via-noop path); only delete/export inherit the base raise.
Behavior preserved: 661 passed / 13 skipped. Docs: backends/README.md rewritten
(three-tier + from_config + callbacks); samples README updated; removed stale
private doc paths.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(memory): 501 on unsupported read/manage endpoints + accurate warm log
Review follow-up on the three-tier MemoryManager refactor.
- Read/manage endpoints (GET /memory, /memory/export, /memory/status,
DELETE /memory, POST /memory/import) and the /memory/reload fallback now
catch NotImplementedError -> 501, matching the fact-CRUD endpoints. The
hasattr->try/except migration had skipped these: they were @abstractmethod
before (every backend implemented them, so they never raised), so once they
became tier-2 default-raise a minimal backend (only add + get_context) hit a
raw 500 -- there is no global NotImplementedError handler. get_memory is
shared via _get_memory_or_501 (covers /memory, export, status, reload
fallback). noop is unchanged: its read-op empty overrides never raise.
- warm() base default returns None (tri-state: True=warmed, False=failed,
None=nothing to warm) so the Gateway lifespan logs "skipping" for a
non-DeerMem backend (e.g. noop) instead of the inaccurate "warmed
successfully" it never earned. DeerMem.warm keeps True/False.
- Tests: 6 router 501 tests (read/manage + reload fallback) + 2 lifespan
warm-log tests (None->skipping, False->warning); conformance/pluggable
assert warm() is None.
705 passed / 13 skipped; lint clean.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(memory): review follow-ups - search-flag consistency, client reload, backend_config purity
Address review feedback on the three-tier MemoryManager refactor:
- [Medium] supports_search/search drift: the invariant now requires the
supports_search ClassVar flag to MATCH whether search() is actually
overridden (type(self).search is not MemoryManager.search), so the flag
can't drift from the impl. Catches both directions at instantiation: a
backend that overrides search() but forgets supports_search=True (was a
misleading tool-mode rejection), and one that sets the flag without
overriding (was a runtime NotImplementedError on the first memory_search).
noop sets supports_search=True to match its search() override. Conformance
adds drift + consistent-backend tests.
- [Low] client.reload_memory fallback: wrap the get_memory fallback so a
minimal backend (only add + get_context) surfaces a clean NotImplementedError
("implements neither reload_memory nor get_memory") instead of an uncaught
propagation -- mirrors the router's 501. Test added.
- [Low] backend_config purity: DeerMem.from_config restores backend_config to
the pure data the host passed after model_post_init parses the injected hooks
into DeerMemConfig (self._config, PrivateAttr); the field stays serializable
(no callables/LLM) and matches the README ("host hooks NOT in backend_config").
Test asserts purity + hooks wired.
- [Low] CHANGELOG: breaking-change note that mode='tool' + non-search backend
now fails fast at startup (was silently empty) so operators recognize it on
upgrade.
- [Nit] .gitignore: drop the env-specific .tmp-pytest/ entry (--basetemp is
local-only, not make test/CI).
709 passed / 13 skipped; lint clean.
Co-Authored-By: Claude <noreply@anthropic.com>
* docs(changelog): correct memory tool-mode fail-fast note
The CHANGELOG entry said mode='tool' + a non-search backend "(e.g. noop)"
fails fast at startup, but noop overrides search() (returns []) and sets
supports_search=True (required by the consistency invariant), so noop IS
search-capable and noop+tool does NOT fail fast. The fail-fast only affects a
custom backend that onboards without overriding search(). Reworded to drop the
misleading noop example and state both shipping backends implement search().
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: Claude <noreply@anthropic.com>
|
||
|
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. |
||
|
March7
|
ce4a6d4c3d
|
fix(backend): remove transient image context after model calls (#4267)
* fix(backend): discard transient image context * fix(backend): protect client image context ids * docs(backend): clarify image checkpoint lifecycle |
||
|
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>
|
||
|
RongfuShuiping
|
4bf028d048
|
feat(memory): add incremental agent-scoped Markdown fact storage (#4279)
* feat(memory): add Markdown fact storage repository * docs(memory): explain storage rewrite for beginners * docs(memory): fix plan markdown formatting * refactor(memory): separate global summaries from agent facts * fix(memory): make Markdown fact updates incremental and safe * Update STORAGE_REWRITE_CHANGES.md * Delete docs/plans/STORAGE_REWRITE_PLAN.md * Delete docs/plans/STORAGE_REWRITE_CHANGES.md * fix(memory): address Markdown storage review feedback * fix(memory): complete review follow-ups * fix(memory): resolve storage review findings * feat(memory): add proactive Markdown migration CLI * fix(memory): harden Markdown storage concurrency * fix(memory): harden markdown storage migration * fix(memory): close migration review gaps --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> Co-authored-by: qin-chenghan <qinchenghan@Huawei.com> |
||
|
Ryker_Feng
|
fa496c0c8d
|
feat(browser): add agentic browser control (#4187)
* feat(browser): add agentic browser control
* fix(frontend): format browser view changes
* fix(browser): keep browser optional and isolate sidecar layout
* fix(browser): address PR review security and IME findings
- Nginx: add a browser-stream WebSocket location before the generic
/api/threads regex so Live upgrades instead of downgrading to HTTP
(both nginx.conf and nginx.local.conf).
- Ownership: require an existing owned thread for the WS stream and REST
navigate, and tear down the browser session on thread deletion so a
later caller cannot reuse a retained page/cookies by guessing the id.
- SSRF: enforce the URL policy at the browser request boundary via a
context-level route guard covering redirects, popups, iframes, and
subresources (skipped for CDP-attached Chrome).
- IME: skip key forwarding while a composition is active so confirming a
CJK candidate with Enter no longer submits the remote page form.
Adds regression tests for the request guard, session teardown on delete,
and the composing-Enter key decision.
* fix(frontend): smooth streaming in long tool threads
* Revert "fix(frontend): smooth streaming in long tool threads"
This reverts commit f0462516eabe77f138d4027ea1c714fb226683cf.
* fix(browser): address review security and lifecycle findings
- Reject cross-origin WebSocket upgrades on the live browser stream
(Origin allow-list reuse of CORS/same-origin helpers) to close a
WS-CSRF hole, and fail closed when the ownership store is absent.
- Warn when a CDP-attached session runs with the SSRF request guard
off, and drop the unreachable CDP screencast teardown dead code.
- Read browser session launch config from a single canonical source
(browser_navigate) so it is deterministic regardless of call order.
- Bound per-thread Chromium accumulation with idle-timeout eviction
and an LRU max-sessions cap.
- Reset the Live reconnect counter on a successful open so the stream
can't permanently stall after the cumulative attempt cap.
* fix(frontend): reduce long tool thread render stalls
Reuse stable historical message groups during streaming, defer heavy Markdown and browser previews, and lazy-decode message images.
* fix(browser): keep live control responsive during continuous input
Why: Manual browser control felt laggy — a physical click ran the remote
Playwright click three times and each non-move input synchronously awaited a
JPEG screenshot, so events queued behind capture (queue wait up to ~237ms).
The first async attempt used a trailing-edge debounce, which froze the visible
page until a wheel/keyboard gesture stopped ("scroll finishes, then it jumps").
What:
- Frontend forwards one `click` per physical click instead of also emitting
`down`/`up`, so the remote page is not clicked twice per gesture.
- Backend detaches live-frame capture from input dispatch: non-move actions
start a rate-limited background refresh loop (leading frame + bounded cadence)
that keeps emitting frames while input continues and never blocks dispatch.
- Add regression tests: input dispatch no longer awaits the screenshot, rapid
inputs coalesce, and continuous input keeps refreshing before it stops.
Scenarios: Verified in the live Browser panel — a single click completes in
~57ms (was blocked behind a 171ms capture), and a 1.14s sustained wheel gesture
renders ~7 frames throughout the scroll instead of one frame after it ends.
* fix(browser): harden worker and session lifecycle
* fix(browser): address latest review feedback
* fix(frontend): preserve optimistic new-chat message
* test(e2e): preserve mocked message run ids
* fix(browser): address capability review feedback
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
|
||
|
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 |
||
|
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
|
474a0fd6b7
|
fix(gateway): correct GitHub auto-retry claim in webhook route docs (#4289)
While investigating a review on PR #4274 (which corrected the same misconception in the channel manager's dedupe-TTL comment), I found a more elaborate version of the same factual error in this route, predating that PR: the docstring claimed "GitHub retries 5xx deliveries with exponential backoff (up to ~5 attempts over ~8 hours)". No such behavior exists. Verified directly against GitHub's documentation: failed deliveries (5xx, timeout, connection error alike) are simply recorded as failed and never automatically redelivered, for both repository and GitHub App webhooks (https://docs.github.com/en/webhooks/using-webhooks/handling-failed-webhook-deliveries). Redelivery is always explicit: the "Redeliver" button, the REST API, or an operator's own recovery script (GitHub's own recommended pattern for the latter runs on a 6-hour cron, illustrating this is opt-in operator tooling, not a GitHub-side mechanism). The 503-vs-200 status code choice itself was already correct and is unchanged: 503 keeps a transient failure recorded as failed so it can be recovered via one of the explicit paths above, while 200 correctly marks permanent/non-retryable conditions (disabled channel, unknown event, missing service) as done so no pointless redelivery is invited. Only the rationale text describing *why* was wrong. - github_webhooks.py: reworded the route docstring and four inline comments/log messages that repeated "GitHub retries" / "so GitHub retries" framing, citing the actual documented behavior. - test_github_webhooks.py: renamed test_dispatch_failure_returns_503_so_github_retries to test_dispatch_failure_returns_503_not_200 and reworded its docstring plus two nearby docstrings/comments with the same framing. No assertions changed - same status codes and response bodies checked. - AGENTS.md: corrected the GitHub Webhooks router table row to match. All 44 tests in test_github_webhooks.py pass, plus the full test_github_dispatcher.py / test_github_channel.py / test_github_registry.py / test_channels.py suites (313 total). ruff check and ruff format --check are clean on both touched Python files. |
||
|
Andrew Chen
|
a9a57fb711
|
fix(gateway): handle null config.configurable in _resolve_thread_id (#4301)
* fix(gateway): handle null config.configurable in _resolve_thread_id
`_resolve_thread_id` read the thread id with
`(body.config or {}).get("configurable", {}).get("thread_id")`. The
`.get("configurable", {})` fallback only applies when the key is absent.
`RunCreateRequest.config` is typed `dict[str, Any] | None`, so a client
can send `{"config": {"configurable": null}}`; the key is then present
with value `None`, and `None.get("thread_id")` raises `AttributeError` —
an unhandled HTTP 500 on `POST /api/runs/stream` and `/api/runs/wait`.
Coalesce the inner value with `or {}` so a null `configurable` yields a
freshly generated thread id, matching the docstring ("or generate a new
one") and the behaviour of `config: null` / `configurable: {}`. The
sibling `_checkpoint_configurable` in thread_runs.py already guards the
same field defensively. Behaviour is byte-for-byte identical for every
input that worked before.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* style: ruff format the new test (lint-backend runs ruff format --check)
Collapse the multi-line assert ruff format flags; behaviour unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Also coerce null configurable in build_run_config, not just _resolve_thread_id
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Daoyuan Li
|
9a5d701355
|
fix(channels): document GitHub dedupe TTL, tighten redelivery tests (#4274)
* fix(channels): document GitHub dedupe TTL, tighten redelivery tests Follow-up to PR #4104's review, whose non-blocking notes were left unaddressed at merge time: - Document the previously-undocumented inbound-dedupe TTL (INBOUND_DEDUPE_TTL_SECONDS, 10 min, in-memory only) directly above the constant in ChannelManager: what it is, why the window, and what happens at the boundary (a manual "Redeliver" clicked after the TTL has elapsed, or any redelivery after a Gateway restart, is not deduped, since _recent_inbound_events is never persisted to ChannelStore). Cross-reference it from the GitHub dispatcher's fan-out comment, which previously implied unconditional redelivery coverage. - test_channels.py::test_github_redelivery_is_deduped_like_other_channels: the _gh helper stamped a 2-part delivery:agent id while production stamps a 3-part delivery:user:agent id. Align the helper with the real shape and add a cross-user assertion the old 2-part shape could not even express. - test_github_dispatcher.py::test_missing_delivery_header_leaves_dedupe_open: previously only asserted the dispatcher-layer id was None. Add the manager-level _is_duplicate_inbound assertion across two header-less deliveries so a future regression that treats a missing id as a real key is caught here too, not only at the dispatcher layer. All four affected test files (test_github_dispatcher.py, test_channels.py, test_github_channel.py, test_github_registry.py) pass; ruff check and ruff format --check are clean. Verified the two tightened tests actually discriminate by temporarily reintroducing each bug they target (2-part id shape; missing-id treated as a real key) and confirming the new assertions fail, then reverting. * fix(channels): correct GitHub redelivery claim in dedupe comments fancyboi999's review on this PR flagged that the previous commit's new comments (manager.py, dispatcher.py) justified the 10-minute dedupe TTL by claiming GitHub automatically retries a failed delivery after its 10-second timeout. GitHub's own documentation says the opposite: failed deliveries (non-2xx, timeout, connection error) are recorded as failed and never automatically redelivered, for both repository and GitHub App webhooks alike. Every redelivery is explicit: the "Redeliver" button, the REST API, or an operator's own recovery script. - manager.py: rework the INBOUND_DEDUPE_TTL_SECONDS comment to state GitHub's actual behavior (no auto-retry), cite the authoritative doc (docs.github.com/en/webhooks/using-webhooks/handling-failed-webhook-deliveries), and justify the 10-minute window as a bound on explicit near-term replays rather than an automatic-retry window. No longer asserts unverified retry behavior for the other channels either. - dispatcher.py: same correction for the dedupe_message_id comment, which previously implied "retried after a timeout" as a distinct, automatic case alongside the manual "Redeliver" button. - test_channels.py / test_github_dispatcher.py: reworded the two docstrings and one section comment that repeated the same "retry-on-timeout" framing, so the corrected mental model doesn't sit next to contradicting prose nearby. No assertions changed. Verified directly against GitHub's current documentation (fetched, not paraphrased from memory): "Handling failed webhook deliveries" states plainly that GitHub does not auto-redeliver; "Automatically redelivering failed deliveries for a GitHub App webhook" / "...for a repository webhook" are both guides for a user-written recovery script (GitHub's own example runs on a 6-hour cron), not a native GitHub feature. No difference in this behavior between GitHub Apps and repository webhooks. All four affected test files pass; the two `test_bot_login_whitespace_only` / `test_trigger_mention_login_whitespace_only` failures in test_github_dispatcher.py are pre-existing on this branch's unmodified HEAD (confirmed against a clean checkout of the same commit) and are unrelated to this change. ruff check and ruff format --check are clean on all four touched files. |
||
|
Aari
|
bc9c027a54
|
fix(uploads): claim the converted markdown filename before writing it (#4288)
* fix(uploads): claim the converted markdown filename before writing it * doc(changelog): record the converted-markdown filename collision fix Covers both surfaces that report markdown_file: the gateway uploads route and DeerFlowClient.upload_files. * fix(uploads): release the claimed markdown name when conversion fails Claiming the companion .md name before conversion means a conversion that writes nothing leaves the name reserved for the rest of the request, so a later same-stem upload is renamed against a name nothing occupies. Uploading notes.docx (conversion fails) + notes.md stored the user's file as notes_1.md with no notes.md on disk, where main kept notes.md. Discard the claim when conversion returns None, at both call sites that pre-claim it. Covers both victims: a later same-stem .md upload, and the next convertible's companion. |
||
|
yym36991
|
0f088033fe
|
fix(gateway): prefer X-Trace-Id over metadata.deerflow_trace_id when header is set (#4283)
When TraceMiddleware binds a valid inbound X-Trace-Id, worker resolution prefers that id over config.metadata.deerflow_trace_id so logs, response headers, Langfuse, and runtime context stay aligned. Also add trace-context marker reset coverage for the inbound-header flag. |
||
|
Daoyuan Li
|
75fa028e89
|
fix(artifacts): serve inline binary artifacts via FileResponse for Range support (#4281)
* fix(artifacts): serve inline binary artifacts via FileResponse for Range support Audio/video/image artifacts that are previewed inline (not downloaded, not active content) were served by reading the whole file into memory and returning it through a plain Response. That response never sets Accept-Ranges and ignores any Range header the browser sends, so seeking an <audio>/<video> element backed by this endpoint always gets the full 200 response back instead of a 206 partial response for the requested byte range -- which is why dragging the seek bar on an audio artifact restarts playback from the beginning instead of jumping to the new position. Route this case through FileResponse instead (as the active-content/ download branch already does), which handles Range/If-Range natively. Verified with a real Range request against the endpoint: initial load now reports Accept-Ranges: bytes, and a ranged GET returns 206 with the correct Content-Range and only the requested slice of bytes. Fixes #3240 * fix(artifacts): address review nits on the inline FileResponse branch Two non-blocking nits from review: - Drop the redundant filename= kwarg on the inline_file FileResponse call. Content-Disposition is already set explicitly on this branch, and FileResponse only uses filename to setdefault that same header -- which is a no-op once it's already present. Harmless today, but removes the latent risk that a future Starlette version turning that setdefault into a hard set would silently flip inline preview to attachment. - Reword the blocking-IO regression-anchor docstring: it claimed awaiting get_artifact for a binary artifact does "zero filesystem IO", but _read_artifact_payload still runs exists/is_file/ mimetypes.guess_type/is_text_file_by_content (an 8 KB read) for binary files too, just offloaded via asyncio.to_thread like the text branch. The gate has nothing to catch because that IO is off the event loop, not because there's none -- reworded to say no full-file read happens, matching the accurate framing already used one paragraph up. tests/test_artifacts_router.py, tests/blocking_io/test_artifacts_router.py (19 tests) are green, and ruff check/format are clean on both changed files. |
||
|
Ryker_Feng
|
a028dfd5fb
|
feat(auth): add keep me signed in login option (#4255)
* feat(auth): add keep me signed in login option * fix(auth): address remember-login review feedback |
||
|
Ryker_Feng
|
e89edb39b1
|
fix(gateway): reject non-positive read limits (#4284) | ||
|
Huixin615
|
c9b6131f8f
|
fix(skills): reload mounted skills without restarting Gateway (#4264)
* fix(skills): add admin-only reload endpoint * fix(skills): preserve cache when reload fails |
||
|
hataa
|
10890e10a8
|
feat(authz): propagate trusted authorization principal context (#4203) | ||
|
AochenShen99
|
94a34f382d
|
feat(context): record effective memory identity per run (#3556)
* feat(context): record effective memory identity per run * fix(context): address memory identity review feedback --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Aari
|
259f51ca4f
|
fix(github): match allow_authors logins case-insensitively (#4218)
* fix(github): match allow_authors logins case-insensitively GitHub logins are case-insensitive, and the sibling gates in this module already treat them that way. allow_authors used a bare string membership test, so a YAML casing mismatch silently dropped owner webhooks that should have bypassed require_mention. * test(github): parametrize allow_authors case-fold over both directions Cover the reverse cfg "alice" / payload "Alice" direction plus the all-caps and exact-case rows from the PR's E2E matrix. Each casing-differs row is red on the pre-fix source; the exact-case row stays green both ways, pinning that case-folding is a superset of the old exact match. |
||
|
Vanzeren
|
1769b2de0d
|
fix: read run stop_reason from runtime context (#4188)
* fix: read run stop_reason from runtime context * fix: address review feedback for #4188 stop_reason integration - migration 0005: use safe_add_column for consistency and drift detection - worker: clear runtime.context stop_reason at start of each _stream_once turn so a clean continuation doesn't inherit a prior cap reason - tests: replace circular unit test with real middleware integration tests that exercise LoopDetectionMiddleware._apply and TokenBudgetMiddleware._apply through the worker, proving the full middleware → runtime.context → persist pipeline * fix(test): resume conftest * fix: stamp stop_reason in all guard middlewares, fix clearing semantics |
||
|
ly-wang19
|
289adcbb02
|
fix(mcp): offload blocking filesystem IO in MCP config update (#3552)
* fix(mcp): offload blocking filesystem IO in MCP config update update_mcp_configuration resolved the extensions config path, probed its existence, read the raw JSON, wrote the merged config, and reloaded it — all blocking filesystem IO on the event loop (PUT /api/mcp/config). The whole read-modify-write after the async admin check has no interleaved awaits, so it moves into one _apply_mcp_config_update helper dispatched via asyncio.to_thread; the masked response is built on the loop. The secret-preserving merge, error codes, and the stdio command allowlist are unchanged. Found via `make detect-blocking-io`. Same class as #3457 / #3529 / #3551. Add tests/blocking_io/test_mcp_router.py anchor, verified red->green under the strict Blockbuster gate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): serialize concurrent config updates with a write lock Address review on #3552: offloading the read-modify-write to a worker thread dropped the implicit serialization the single-threaded event loop provided, so two concurrent PUT /api/mcp/config calls could interleave and clobber each other. Guard the offloaded RMW with a module-level asyncio.Lock to restore within-process atomicity (cross-process writers remain a separate, pre-existing concern). Add a serialization regression test (red->green: without the lock the tracked max concurrency exceeds 1). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: address mcp config blocking io review --------- Co-authored-by: ly-wang19 <ly-wang19@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Yufeng He
|
a0acdda103
|
fix(gateway): keep create_thread idempotent when the insert loses a race (#3800)
* fix(gateway): keep create_thread idempotent when the insert loses a race The /api/threads create endpoint checks for an existing row and then inserts, but the two steps are not atomic. When two requests create the same thread_id concurrently, one commits first and the other's INSERT fails on the duplicate primary key. The SQL-backed thread store raises, the handler catches it as a generic failure, and the loser gets a 500 — even though the docstring promises the call is idempotent. Re-read the row after a failed insert: if it is now present (the competing request won), return it instead of surfacing the conflict. A genuine write failure where the row is still absent keeps the 500. Covered by a regression test that simulates the lost race and asserts the endpoint returns the existing thread rather than erroring. Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com> * fix(gateway): scope create_thread race recovery and mirror owner reconciliation Address review on #3800: - Scope the insert-race recovery to sqlalchemy IntegrityError instead of a broad `except Exception`, so a non-race failure that coincides with an existing row is no longer silently returned as a 200. Any other error logs and surfaces as a 500. (The memory store overwrites on duplicate rather than raising, so it never reaches this path.) - Route both the fast path and the recovery path through a shared `_resolve_existing_thread` helper so the recovery performs the same trusted- owner reconciliation (claiming a legacy unscoped `user_id=None` row) the fast path does. Thread ownership no longer diverges based on which path resolved the record. Add regression tests for both: the recovery claims an unscoped row for a trusted owner, and a non-IntegrityError failure surfaces as a 500. --------- Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
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. |
||
|
Daoyuan Li
|
2a7469cdbc
|
fix(channels): dedupe GitHub webhook redeliveries (#4104)
* fix(channels): dedupe GitHub webhook redeliveries The inbound dedupe added for the IM channels in #3584 keys ChannelManager._is_duplicate_inbound on a top-level metadata["message_id"] plus a workspace id. The GitHub channel added later in #3754 never populated either: the X-GitHub-Delivery GUID was buried in metadata["github"]["delivery_id"] and no workspace id was set, so every GitHub delivery produced a None dedupe key and was never deduped. GitHub reuses the same X-GitHub-Delivery GUID when a delivery is retried after a timeout or replayed via the repo/App "Redeliver" button, so a redelivery re-ran the agent with real side effects (e.g. a duplicate PR comment) while the other channels absorbed an identical redelivery. fanout_event now stamps the dedupe identity the manager already consumes: - workspace_id = repo (globally unique, always present; mirrors Telegram/WeChat keying the workspace on the chat id) - metadata["message_id"] = f"{delivery_id}:{agent.name}" A single delivery fans out to N agents, so the id is scoped to (delivery, agent): an identical redelivery reproduces the same pairs (deduped) while two agents matching the same delivery keep distinct ids and both still fire. When the delivery header is absent the id is left None, so the manager fails open exactly as before. Tests: dispatcher-level coverage that the identity is stamped, stable across redelivery, and distinct per agent/delivery; a ChannelManager regression that an identical GitHub redelivery dispatches once while a new delivery and a second agent on the same delivery still fire. * fix(channels): scope GitHub dedupe id by owning user _inbound_dedupe_key indexes on (channel, workspace_id, chat_id, message_id). For GitHub, workspace_id and chat_id are both the repo, so the owning user was never represented anywhere in the key - fanout_event stamped the id as f"{delivery_id}:{agent.name}". Two different users each binding an agent of the same name (e.g. "reviewer") to the same repo+event therefore produced an identical dedupe id for both fan-out messages. ChannelManager._is_duplicate_inbound treated the second as a replay of the first and silently dropped it, even though GitHub delivered the webhook once and both users' agents legitimately matched. Fold match.user_id into the id: f"{delivery_id}:{match.user_id}:{agent.name}". Genuine redeliveries (same user, same agent, same delivery) still produce the same id and are deduped; two agents - same-named or not, same user or not - on one delivery now always keep distinct ids. Tests: new cross-user regression pins that two users' same-named agents both dispatch instead of the second being deduped against the first; the existing per-(delivery, agent) test and the literal id assertion in test_delivery_id_populates_inbound_dedupe_identity are updated for the new id shape. * fix(channels): point cross-user dedupe comment at its regression test Replace the inline reviewer-handle attribution with a pointer to test_dedupe_identity_distinguishes_same_agent_name_across_users, which ages better than a person's name in committed code. |
||
|
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> |
||
|
Daoyuan Li
|
0dd90ccfde
|
fix(agents): require config.yaml in update_agent's legacy-agent guard (#4166)
update_agent (the harness tool) and PUT /api/agents/{name} (the same
operation over HTTP) share an identical guard meant to block updates to
an agent that only exists in the legacy shared layout. The guard checked
bare directory existence:
if not agent_dir.exists() and paths.agent_dir(name).exists():
When memory is enabled, the first time a user chats with a legacy shared
agent, the memory writer creates a per-user directory containing only
memory.json (no config.yaml). agent_dir.exists() is then true, so the
guard never fires: the tool falls through to load_agent_config, which
resolves through to the legacy shared config via the already-hardened
resolve_agent_dir, and silently writes a brand-new config.yaml/SOUL.md
into the memory-only directory. That forks the agent for just this user;
every other user keeps reading the original shared config forever, with
no error or warning.
resolve_agent_dir itself was already hardened against exactly this
failure mode: it requires config.yaml to exist, not just the directory.
Mirror that condition at both call sites here.
|
||
|
Daoyuan Li
|
51bb19fa1b
|
fix(channels): drop redundant GitHub review-comment webhook fan-out (#4131)
* fix(github): drop redundant pull_request_review_comment fan-out noise
GitHub fires one pull_request_review_comment webhook per inline comment
attached to a pull_request_review submission, in addition to the single
pull_request_review event for the review itself. A bot reviewer like
CodeRabbit commonly leaves 20-30 inline comments per review, flooding
the webhook with near-duplicate deliveries that carry nothing an agent
doesn't already have -- it fetches every inline comment itself via
`gh api` when it processes the parent pull_request_review event.
Filter these out in fanout_event() before the registry lookup / per-
agent loop, since the redundancy is a property of the event itself, not
of any specific agent binding. A companion comment is identified by
pull_request_review_id being set (it belongs to a review) and
in_reply_to_id being absent (it is not itself a reply within an
existing thread -- that case is a genuine new interaction and must
still fire).
* fix(channels): scope review-comment suppression to bindings that also see the review
The redundant pull_request_review_comment filter suppressed every
companion comment unconditionally, before the registry lookup even ran.
That premise only holds for a binding that also subscribes to
pull_request_review on the same repo -- events are opt-in per binding,
so a binding registered for pull_request_review_comment alone never
receives the parent review event and the companion comments were its
only delivery of the review's inline content. Suppressing those too was
a silent, total loss for that binding, not noise reduction.
Move the check into the per-agent loop so it only suppresses a matched
binding's companion comment when that same binding also has an active
pull_request_review trigger on this repo, reusing the existing registry
lookup rather than re-walking bindings by hand. Bindings subscribed to
pull_request_review_comment alone now always fire, matching the
opt-in-per-binding contract documented in triggers.py.
* fix(channels): close require_mention gap in review-comment redundancy gate
willem-bd's second review round found that the per-binding redundancy gate
(commit b3791b33) treated a binding as "covered" purely by checking whether
it also registers a `pull_request_review` trigger on this repo, ignoring
whether that trigger itself requires a mention. A `pull_request_review_comment`
payload never carries the paired review's own top-level body, so there is no
way to verify from a comment delivery whether that trigger's own
`require_mention` check would actually pass. A human `@mention` living only
in one inline comment (not the review summary) could therefore be lost
twice: the review event filtered out by its own `no_mention` gate, and the
one inline comment that carries the mention dropped here as "redundant" --
the same silent-loss shape as the original bug, through a narrower path.
The gate now also requires the paired `pull_request_review` trigger's
resolved `require_mention` to be false before treating it as coverage,
trading a small amount of residual redundancy (an extra companion delivery
when the review would have fired anyway) for zero silent loss.
While in the same code, also addressed two smaller review notes:
- The redundant-comment skip reason now prefers the companion's own trigger
verdict when that verdict is also a skip (e.g. its own `require_mention`
independently fails), instead of always reporting the generic
`redundant_review_comment` label.
- `_pr_review_prompt` now tells the agent to fetch a review's inline
comments via `gh api .../reviews/{id}/comments` -- the redundancy gate's
suppression is only genuinely redundant if the agent actually recovers
that content from the parent review event, and nothing previously told it
to (zhfeng's review).
Tests: reproduces willem-bd's exact scenario (dual-subscribed binding,
require_mention on the review trigger, mention present only in the inline
comment) and confirms it fails on the prior code and passes with the fix;
adds a multi-agent-independence test and a skip-reason-precedence test;
adds prompt tests locking the new fetch-hint text and its missing-id guard.
Fail-before/pass-after verified via patch-file revert (not stash, to avoid
colliding with sibling worktrees).
|
||
|
Vanzeren
|
13fd8e229a
|
fix(checkpoint): persist run duration in checkpoints for history reads (#4118)
* fix: persist run duration in checkpoints for history reads * fix(checkpoint): harden run duration persistence * fix(checkpoint): persist run durations in metadata * fix(checkpoint): address review findings for run duration persistence - Add valid_duration_entry() shared validation helper (worker.py) - Rename _persist_run_durations -> persist_run_durations as public API - Import public persist_run_durations and valid_duration_entry in threads.py - Use BackgroundTasks for lazy backfill write to avoid blocking history reads - Add TODO about O(runs) growth of run_durations in checkpoint metadata - Document REGENERATE_HISTORY_RAW_SCAN_LIMIT doubling assumption * fix(checkpoint): replace pruning TODO with justification Accumulated run_durations overhead (~50 bytes/run_id) is negligible compared to messages channel blobs; no pruning strategy is needed. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
AnoobFeng
|
446fa03801
|
fix(context): resolve context compress bug (#4065)
* fix(runtime): persist original human input outside model sanitization * refactor(history): load thread messages by global event sequence * fix(frontend): make summarization rescue a transient history bridge * fix(frontend): old message not append tail 1. add identity anchor 2. add bridgeOrder * fix(frontend): lint error fix * fix: address review feedback and harden pagination coverage - defer transient history ref writes until after render commit - cover large middleware-only history scans - verify infinite-query refetch recalculates page cursors - document AI event types and anchor-weaving differences * fix: harden message pagination and enrichment - append unmatched live tails after canonical history - warn and stop when pagination has_more lacks a cursor - deep-copy restored UI messages to isolate model-facing content - log invalid event sequence and non-advancing cursor errors - pass user_id explicitly through event-store history queries - cover middleware-only AI runs across memory, JSONL, and DB stores * fix: address pagination review feedback * fix(frontend): checkpoint has unknow redener content, optimize the anchor policy * fix(frontend): unit test issue missed previously, remove the TanStack cache trimming * fix(gateway): harden message history queries and provenance - reject externally forged original_user_content metadata - validate provenance metadata in upload and sanitization middleware - make run lookups fail closed by default - batch feedback queries by run ID - align memory message filtering with persistent stores |
||
|
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> |
||
|
heart-scalpel
|
b53c1ae0e0
|
fix(runs): cancel degrades to lease takeover for multi-worker (#4064)
* fix(runs): cancel degrades to lease takeover for multi-worker Work item 4 of the multi-worker ownership epic (https://github.com/bytedance/deer-flow/issues/3948). Problem: POST /runs/{run_id}/cancel landing on a non-owning worker returns 409 — the cancel button silently fails under GATEWAY_WORKERS>1 with no sticky routing. cancel() required the current worker to hold the in-memory task/abort_event, which any non-owner pod cannot satisfy. Changes: - RunManager.cancel() returns CancelOutcome enum (cancelled / taken_over / lease_valid_elsewhere / not_active_locally / not_cancellable / unknown) instead of bool, so the router can map each outcome to the right HTTP response. - New store primitive claim_for_takeover(): a single atomic conditional UPDATE that marks a run as error only when status IN (pending, running) AND (lease IS NULL OR lease < now - grace). Closes the stale-read / concurrent-heartbeat race — if the owner renews between our read and write, the UPDATE matches 0 rows and we surface lease_valid_elsewhere. - HTTP cancel + stream-join endpoints route on CancelOutcome: cancelled -> 202 (or 204 with wait=true); taken_over -> 202 immediately (no SSE streaming — the run is terminal on another worker, streaming would hang); lease_valid_elsewhere -> 409 + Retry-After header computed from lease_expires_at + grace_seconds. - RunManager.grace_seconds exposed as a public property; the router no longer reaches into _run_ownership_config. - _is_lease_expired extracted to a module-level function, shared by RunManager.cancel() and MemoryRunStore.claim_for_takeover(). - GATEWAY_WORKERS=1 + heartbeat_enabled=false is zero-regression: the non-local path short-circuits to not_active_locally, preserving the original 409 behaviour the existing tests pin. Tests: 12 new (5 store primitive + 4 cancel-takeover unit + 3 HTTP including a regression guard verifying POST /stream?action=interrupt on a dead-owner run returns 202 instead of hanging on SSE). 244 directly-related tests pass; 36/36 blocking-IO gate pass. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(runs): guard update_status and self-terminate on takeover Two defenses close a split-brain window where the original owner could overwrite a peer's takeover status: - update_status (SQL + memory store) now guards on status IN ('pending','running'). When takeover already set the row to 'error', the owner's final status write matches 0 rows and is dropped. - _persist_status: when update_status returns False, check whether the row exists before attempting recovery via put(). If the row exists (takeover by another worker), skip recovery instead of blindly upserting over the takeover. - Heartbeat _renew_leases: when update_lease returns False (row no longer pending/running or owner changed), cancel the local task so wasted CPU is bounded to the next heartbeat tick (~10s) instead of the full task lifetime. Also fix three reviewer feedback items: - Re-fetch the store row when cancel() returns lease_valid_elsewhere, so Retry-After uses the owner's freshly-renewed lease instead of a stale value from request start. - Fallback 'unknown' in takeover error message when owner_worker_id is NULL (pre-ownership data). - Remove dead else-10 branch from grace_seconds property (unreachable — all callers are downstream of the heartbeat_enabled guard). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * test(runs): pin split-brain defences from update_status guard + heartbeat Three tests lock down the takeover authoritativeness so a late-running owner cannot overwrite a peer's claim: - update_status must reject writes when the store row is already terminal (taken over by another worker). - _persist_status must skip row-recovery via put() when the row exists but has been taken over. - Heartbeat _renew_leases must cancel the local task when update_lease returns False (row claimed by another worker). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(runs): precise outcome + log when local cancel loses to peer takeover Two reviewer precision nits on the split-brain defence: - _persist_status: branch the skip-reason log on existing["status"]. error → WARNING "peer takeover" (anomalous); interrupted/success → INFO "local cancel/completion race" (expected when user hits stop as the run finishes). Stops noisy false-positive takeover warnings in operator logs. - cancel() local path: when _persist_status returns False, re-check the store. If a peer's claim_for_takeover flipped the row to error between our in-memory cancel and the guarded update_status, surface taken_over instead of cancelled so the client sees a status consistent with the store. Test: test_cancel_returns_taken_over_when_peer_claims_during_local_cancel pins the race outcome. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(runs): widen update_status guard, de-duplicate lease helpers, add coverage Round 3 of reviewer feedback: - Widen update_status guard to status IN ('pending','running','interrupted'). The original guard blocked interrupted→error (the rollback finalize path), losing the "Rolled back by user" message. interrupted is now permitted while error/success stay locked — takeover protection unchanged. - claim_for_takeover False now re-reads the store row to distinguish causes: owner renewed lease → lease_valid_elsewhere; row went terminal → not_cancellable; another worker already took it over → taken_over. - Extract _raise_lease_valid_elsewhere() helper to de-duplicate the 409+Retry-After block shared across cancel_run and stream_existing_run. - Extract _lease_expired_or_null() in persistence/run/sql.py to de-duplicate the lease-expiry SQL WHERE clause shared by claim_for_takeover and list_inflight_with_expired_lease. - 11 new tests: 5 SQL-layer claim_for_takeover (expired/valid/NULL/ terminal/nonexistent), 3 _compute_retry_after unit (NULL/unparseable/ normal), 2 claim re-read precision (terminal/takeover), 1 stream endpoint 409+Retry-After. Not addressed (non-blocking, reviewer agreed): - The 2–3 store.gets in the takeover cold path: optimizing the API to accept a pre-fetched record would couple the router to the manager more tightly than justified by the perf gain. - The lease-expiry inline loop in MemoryRunStore.list_inflight_with_- expired_lease pre-computes cutoff once for all rows; switching to the shared _is_lease_expired helper would recompute datetime.now() per row with no real benefit. 260 related tests pass; 36/36 blocking-IO gate pass; ruff clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(runs): de-duplicate lease-expiry helper, restore defensive fallback Address final round of review feedback: - Extract is_lease_expired to deerflow.utils.time (no _ prefix, public utility). Manager and MemoryRunStore now import from the same place instead of the store reaching backward into the manager for a private function. - Restore defensive else-10 fallback in grace_seconds property (removed in an earlier round). The guard is unreachable for current callers but protects future ones from AttributeError. - Comment the transient in-memory interrupted vs store error state when a local cancel is superseded by a peer takeover. - Comment the max(1, ...) floor in _compute_retry_after — the floor is a lower bound, not a poll interval; clients should apply jitter. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: rayhpeng <rayhpeng@gmail.com> |
||
|
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. |
||
|
Daoyuan Li
|
88b0484898
|
fix(channels): validate channel provider before resolving its config (#4100)
`_provider_config` resolved a request-supplied provider name with an
unallowlisted `getattr(config, provider, None)`. `ChannelConnectionsConfig`
carries non-provider attributes -- the `enabled` and `require_bound_identity`
bool fields plus the `provider_status` method -- so a name matching one of
them returned that attribute instead of falling through to the intended 404.
Callers (e.g. `POST /api/channels/{provider}/connect`, reachable by any
authenticated user) then dereferenced the bool/method as a provider config,
crashing with `AttributeError` -> HTTP 500.
Validate `provider` against the `_PROVIDER_META` allowlist before the lookup,
matching how `_credential_fields` / `_connect_instruction` / `_connect_url`
already gate provider names, so unknown providers get a clean 404.
Add a parametrized router regression test covering `enabled`,
`require_bound_identity`, `provider_status`, and an unknown name.
|
||
|
d33kayyy
|
8ade23d779
|
fix(artifacts): honor trusted owner-user-id header (#3982)
* fix(artifacts): honor trusted owner-user-id header The artifact endpoint resolved paths only via the effective user, so a trusted internal caller acting on behalf of an owner (carrying X-DeerFlow-Owner-User-Id) read the synthetic internal user's storage and 404'd on files the owner's run had written. Resolve the owner via get_trusted_internal_owner_user_id and pass it through resolve_thread_virtual_path (now accepting an optional user_id), matching the memory and threads routers. Browser/API callers send no such header and fall back to the effective user, so their behavior is unchanged. * fix(artifacts): normalize trusted owner id through make_safe_user_id The owner-user-id header carries the raw platform owner id, while runs store files under the make_safe_user_id bucket. Resolve artifacts with the normalized id, mirroring the memory router. |
||
|
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> |
||
|
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 |
||
|
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. |
||
|
Ryker_Feng
|
26d7a5970d
|
feat: add manual context compaction (#3969)
* feat: add manual context compaction * fix: harden manual context compaction |
||
|
sqsge
|
927b833ef4
|
fix(guardrails): propagate internal owner attribution to guardrail context (#3839)
* fix guardrail attribution for internal owner runs * fix guardrail owner attribution mismatch |
||
|
heart-scalpel
|
8cde7f258e
|
feat(gateway): refuse multi-worker startup on non-Postgres backends (#3960)
Why: issue #3948 identifies four correctness breakers when GATEWAY_WORKERS > 1. Work item 1 adds a startup gate that refuses to boot when database.backend is not postgres, giving operators a clear error instead of silent SQLite write-lock corruption. The gate runs inside langgraph_runtime() before init_engine_from_config, so a misconfigured deploy never opens a listener or writes to disk. Non-integer env values fall back to 1 so uvicorn's own validation is unaffected. |
||
|
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>
|
||
|
Ryker_Feng
|
186c6ea463
|
feat: add branching support for assistant turns (#3950)
* feat: add assistant turn branching * fix(threads): skip workspace clone when branching from historical turn Workspace files are not checkpointed, so cloning them onto a branch rooted at an older assistant turn leaked files created in a later timeline. Restrict the best-effort workspace copy to branches taken from the latest turn; historical-turn branches now report workspace_clone_mode="skipped_historical_turn" and keep only the restored message history. * style(frontend): fix prettier formatting in e2e mock-api Collapse the branch-title normalization chain onto a single line to satisfy the frontend lint (prettier --check) CI gate. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Ryker_Feng
|
00161811bd
|
feat: add workspace change review for agent runs (#3945)
* feat: add workspace change review * chore: format workspace change files * fix: optimize workspace change summaries * style: refine workspace change badge scale * fix: restore workspace change user context import * fix(frontend): gate workspace change badge to assistant messages Only pass run_id to assistant MessageListItems so the workspace-change badge can never render under a user's prompt, which carries the same run_id. Assert single badge render in the E2E flow. * fix(workspace-changes): address review feedback on diff parsing and badge - Restrict unified-diff header detection to "+++ "/"--- " (trailing space) in both backend _count_diff_lines and frontend getWorkspaceChangeLineClass so content lines beginning with +++/--- are counted/styled correctly - Gate WorkspaceChangeBadge to ai messages so tool messages folded into an assistant group don't render a duplicate badge - Add regression tests for both diff-classification fixes - Apply prettier formatting to workspace-changes files flagged by CI --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Xinmin Zeng
|
4d660b202a
|
feat(skills): bind request-scoped secrets for autonomously-invoked skills (A+) (#3938)
* feat(skills): bind request-scoped secrets for in-context (autonomously invoked) skills Extends the #3861 binding point A (slash-activation only) to A+: the injection set is recomputed on every model call from two unioned sources — the run's most recent slash activation (persisted on the run context so the tool loop keeps the binding) and skills the model actually loaded in this thread (ThreadState.skill_context), re-validated against the live registry each call. Authorization stays three-gated regardless of activation style: skill enabled by the operator, values supplied per-request by the caller in context.secrets (never persisted server-side, never from the host env), names declared in the skill's required-secrets frontmatter. Because the set is replaced per call, eviction from skill_context or a caller that stops supplying a value revokes injection on the next call. New frontmatter field secrets-autonomous (default true) lets a skill restrict binding to explicit slash activation; malformed values fail closed to false. Binding changes are recorded as a middleware:skill_secrets journal event carrying names only. Design informed by a survey of peer systems (Claude Code, Codex CLI, opencode, pi, deepagents, hermes-agent, QwenPaw) and specs (agentskills.io, MCP 2025-11-25): the industry trust boundary is enable-time consent plus caller-scoped credentials, not per-invocation ceremony; no surveyed system scopes secrets to an activation turn. Part of #3914 * refactor(skills): centralize secret context keys, document intentional per-call reload Review follow-ups (no behavior change): move the two private binding keys (__slash_skill_secret_source, __skill_secrets_binding_audit) into secret_context.py and add them to REDACTED_CONTEXT_KEYS so the redaction allowlist stays a complete guard even though both keys hold names only. Document why _in_context_secret_sources reloads skills every call rather than caching: load_skills re-reads enabled state so an operator disabling a skill revokes its binding on the next model call — an mtime cache would miss enable/disable toggles and keep injecting after a disable. * fix(skills): match in-context secret bindings by path only, never by name Review finding (confused deputy): _in_context_secret_sources fell back to name matching when a skill_context path did not resolve. DeerFlow lets a custom skill shadow a same-named public/legacy one (load_skills de-dupes by name, custom wins), so a thread that read public/foo could bind the custom foo's declared secrets although the custom skill was never loaded in the thread. The recent user-isolation path changes make by-path misses (and thus the dangerous fallback) more likely. Drop the by-name fallback: match strictly by the exact container file path the model read; an unresolved path simply does not bind (the safe direction). Regression tests cover the shadowing case and a stale path. Part of #3914 * fix(skills): resolve secret-binding sources via registry; strip caller __-keys Security review (willem-bd, #3938): 1. Forged `__slash_skill_secret_source` bypassed the enabled/allowlist/ secrets-autonomous gates. runtime.context is caller-mergeable, and the slash source was trusted as authoritative (its stored requirements were injected directly). Now the slash source records only the activated skill's canonical container path, and BOTH the slash and in-context sources resolve the live registry skill by normalized path each call (_resolve_registry_skill) — binding only that real, enabled, allowlisted skill's own declared secrets. A forged path resolves to nothing. As defense in depth, build_run_config strips caller-supplied __-prefixed context keys at the gateway boundary. 2. Malformed caller requirements crashed the run (unguarded tuple unpack / DoS). The middleware no longer unpacks caller-provided requirement data at all — declarations come from the registry — so a malformed source fails closed instead of raising. 3. Path-normalization asymmetry silently disabled in-context binding on a trailing-slash container_path config. Both the registry keys and the lookup path are now posixpath.normpath'd. Regression tests: forged source rejected, forged-but-real path ignores caller requirements + allowlist, malformed source fails closed, trailing- slash config binds, gateway strips __-keys. Part of #3914 * docs(skills): correct _SLASH_SECRET_SOURCE_KEY comment and note fail-closed trade-off Post-review cleanup: the key now stores only the canonical container path (the comment still described the pre-fix skill-name+requirements shape), and document that a transient registry-load failure fails closed (drops the binding for that call) rather than trusting stale data. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
codeingforcoffee
|
4669d3c089
|
feat(gateway): cache-aware cost accounting (#3920)
* feat(gateway): cache-aware cost accounting + /api/console observability endpoints - Capture prompt-cache hits (usage_metadata.input_token_details.cache_read) in RunJournal and SubagentTokenCollector as a sparse cache_read_tokens key in token_usage_by_model (JSON field — no schema migration; legacy bucket shapes unchanged) - New read-only /api/console router: GET /stats (headline counters), GET /runs (cross-thread paginated history joined with thread titles), GET /usage (zero-filled daily token series + per-model breakdown); user-scoped, 503 on the memory database backend - Optional models[*].pricing (currency, input_per_million, output_per_million, input_cache_hit_per_million) powers real spend estimation; cache-hit input tokens are billed at the hit price (omitted hit price falls back to the miss price as a conservative upper bound); unpriced models yield cost: null - create_chat_model strips the presentation-only pricing block so it never reaches the provider client (unknown kwargs are forwarded into the completion payload and break live calls) - Tests: console router SQLite round-trips, journal/collector cache capture incl. a DeepSeek raw-usage pin test, factory strip regression Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor: address review feedback on cost sum and sparse cache_read_tokens - console.py: replace the walrus-in-generator total-cost sum with an explicit loop (review noted the multi-line form reads ambiguously) - token_collector.py: omit cache_read_tokens from usage records when the provider reported no cache hits, matching the journal's sparse per-model bucket shape; absent is treated as 0 downstream - add a regression test pinning the sparse record shape Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: coffeeFish <codeingforcoffee@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |