mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-07-27 08:28:00 +00:00
114 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Huixin615
|
090e80c1dd
|
fix(runtime): fail-stop runs when lease ownership cannot be confirmed (#4431)
* fix(runtime): fail-stop runs after lease expiry * test(runtime): cover late successful lease renewal --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Huixin615
|
1cd5dea336
|
fix(streaming): signal replay history gaps (#4426)
* fix(streaming): signal replay history gaps * fix(streaming): guard initial Redis replay window * fix(frontend): align inactive gap recovery --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Aari
|
244ce7739f
|
fix(runtime): linearize delta-mode checkpoint resume (#4460)
* fix(runtime): linearize delta-mode checkpoint resume Resuming a run from an older checkpoint forks the lineage, and in delta mode that fork's state cannot be materialized correctly: the delta history walk collects every pending_writes entry stored on each on-path ancestor, but a shared parent also carries the writes of the sibling child that was abandoned. Those writes replay into the fork, so the run starts from a message list that still contains the answer it was meant to replace — regenerating in a branched thread surfaced this as the superseded assistant message reappearing beside the new one after a reload. All three saver implementations are affected, so write-to-child ownership is a gap in the upstream delta contract rather than one saver's slip. Rather than reimplement that walk, express the fork as what it means: materialize the requested checkpoint's state, write it as an Overwrite on the current head (which has no siblings), and run linearly. The abandoned turn stays in history as the rewritten head's ancestry. This runs after the rollback point is captured, so cancel-with-rollback still restores the real pre-run head, and fails closed — an unreadable resume checkpoint raises instead of falling back to the corrupt fork. Full mode keeps forking: its checkpoints carry complete channel_values and need no replay. * fix(runtime): restore complete delta resume state * fix(runtime): linearize delta rollback restoration * Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix(runtime): serialize delta resume preparation --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> |
||
|
DanielWalnut
|
bb9f67aaf1
|
fix(runtime): close cancelled replacement admission (#4472) | ||
|
Vanzeren
|
1c7531242c
|
feat(runtime): record terminal artifact delivery receipts (slice 1 of #4272) (#4365)
* feat(runtime): record terminal artifact delivery receipts (#4272) * fix(runtime): persist delivery receipts across recovery * test(runtime): cover delivery receipt invariants * fix(runtime): preserve terminal status on receipt outages |
||
|
Zhengcy05
|
2f60bee388
|
fix: surface length-capped model responses (#4309)
* fix: surface length-capped model responses * fix: avoid the influence of the mid-turn * fix: correcting semantic annotations * fix: add ModelLengthTerminationDetector to compatible providers * fix:delete redundancy code * fix:supplementing log information improves observability * fix: align the document and complete the assertions. * fix: unit test * fix: revert AGENTS.md * fix: unit test * fix: add annotation and skip AIMessage has empty content |
||
|
Aari
|
68797c5759
|
fix(gateway): scope branch history seed run ids per inherited turn (#4459)
Branch creation seeds the new thread's run-event feed from its checkpoint so inherited history survives the first run (#4380). Every seeded row carried one shared run id, but run_id is a *turn* identity to the feed's consumers, not a provenance tag: regenerating the inherited answer resolves that row's run id as the superseded source, and GET /messages/page then drops every row carrying it. One shared id for the whole seed therefore deleted the complete inherited history on a branch's first regenerate, leaving only the regenerated turn. Group seeded rows into one synthetic run per inherited turn (branch-seed-{thread_id}-{n}), a new turn opening at each persisted human message — the same boundary a real run has, including the allowlisted hidden ask_clarification reply, which resumes as its own run. Supersession is then confined to the turn actually regenerated. Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Aari
|
37c343fe30
|
fix(summarization): summarize with the run model, fall back on summary-provider failure (#4361)
* fix(summarization): own the run model for compaction; bound failure With summarization.model_name: null the summary model resolved to config.models[0] while the executing model is selected per run; when they differ and models[0]'s provider is broken (expired key, quota, outage) compaction silently failed every triggered turn and context grew unbounded until the main provider 400s the run (#3103's shape), even though the run's own model was healthy. Model ownership is now sourced from the builders, not re-derived at runtime: - The lead, subagent, and manual /compact builders each pass the resolved run model into create_summarization_middleware(run_model_name=...). The middleware no longer reads runtime.context / get_config(), which do not carry a custom agent's or a subagent's resolved model, so a custom-agent lead run and a distinct-model subagent now summarize with their own model, not models[0] / the parent's. Runtime re-resolution and the per-name model cache are removed. - model_name: null summarizes with the run's own model; an explicitly configured summary model generates and falls back to the run model on failure. The fallback is built lazily after the primary fails and its construction is guarded, so a broken fallback cannot skip a healthy primary or escape the automatic failure boundary. Failure is bounded and side-effect-safe: - An empty or whitespace-only response is treated as a generation failure, not a valid summary, so compaction never removes all history for an empty replacement. - compact_state/acompact_state take raise_on_failure independent of force: the manual /compact path always surfaces a generation failure (even force=false) and routes it to the existing ContextCompactionFailed path (HTTP 500 -> frontend error toast) instead of an unconsumed response reason. The automatic path leaves compaction state unchanged. - before_summarization hooks fire only after a replacement summary exists. SummarizationConfig.model_name, config.example.yaml, and docs/summarization.md document the final lead/subagent/manual ownership rules. Part of RFC #4346 (section A). Evaluating fraction/triggers against the run model's profile (profile ownership) is a separate follow-up. * fix(summarization): manual /compact model ownership + fail-open construct/parse Manual /compact carried only agent_name, so it derived the run model from the custom-agent model or config.models[0] and missed the request-selected model the run path uses (request -> custom-agent -> default). Carry model_name through ThreadCompactRequest and the frontend compact call, resolve with the same precedence, and move the custom-agent config read off the event loop (asyncio .to_thread) with user_id so the strict blocking-IO gate is not bypassed by the broad except. Make one summary attempt own its full lifecycle so the fail-open boundary covers construction and response parsing, not just invocation: build each candidate model lazily and guarded (a raising constructor falls through to the healthy run model instead of breaking agent construction), build the model_name:null primary from the run model rather than config.models[0], and run response text extraction inside the invocation try so a failing .text accessor falls back instead of escaping compaction. Adds factory-level constructor-failure, response-extraction-failure (sync/async), and route-path model-ownership tests. |
||
|
MiaoRuidx
|
735f67a5b2
|
fix: guard pending run startup cancellation (#4450)
* fix: guard pending run startup cancellation * fix(run): address startup review feedback * fix(run): narrow start_run store contract --------- Co-authored-by: MiaoRuidx <12540796+MiaoRuidx@users.noreply.github.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Huixin615
|
8af760fc30
|
fix(runtime): make orphan reconciliation lease-aware (#4427) | ||
|
Vanzeren
|
3c8b82c594
|
fix(runtime): serialize checkpoint writes with active runs (#4437)
* fix(runtime): serialize checkpoint writes with active runs * fix(runtime): address checkpoint reservation reviews * fix(runtime): address reservation race reviews * fix(runtime): refine reservation conflict semantics |
||
|
H Haidong
|
c7538cfb35
|
fix(runs): terminate orphaned streams after lease recovery (#4420)
* fix(runs): terminate orphaned streams after lease recovery * fix(runs): include recovered ids in callback warnings * fix(runs): harden orphan recovery lifecycle |
||
|
ShitK
|
a4ede80deb
|
fix(runtime): reject unsupported run options and stream modes (#4430)
* fix(runtime): reject unsupported run options * fix(runtime): align SDK run compatibility * fix(frontend): avoid unsupported events stream mode --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
MiaoRuidx
|
80c06414f8
|
fix: make orphan reconciliation lease-aware (#4434)
让启动/孤儿 run 恢复在最终写入前通过 claim_for_takeover 原子重查 lease，避免 owner 在扫描后续约成功仍被误标为 error。 补充扫描后续约的回归测试，并把 reconciliation 写失败测试迁移到 takeover claim 路径。 Co-authored-by: MiaoRuidx <12540796+MiaoRuidx@users.noreply.github.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Aari
|
5f0108f56c
|
fix(runtime): stop subgraph stream frames impersonating root frames (#4407)
* fix(runtime): stop subgraph stream frames impersonating root frames The web frontend always requested stream_subgraphs, and since delegated subagent graphs inherit the parent checkpoint namespace (#4215), their values snapshots and token chunks ride the parent stream. The worker's _unpack_stream_item dropped the namespace and published every subgraph frame under a bare event name, so a subagent's values snapshot replaced the whole thread view in SDK clients (#4399), its token chunks flooded the parent message stream, and a subagent's LLM error fallback could be mistaken for the parent run's. Publish subgraph frames under namespace-qualified SSE event names (mode|ns1|ns2, LangGraph Platform style) and keep root-only consumers (file-tool chunk batcher, subagent event persistence, error-fallback detection) on root frames only. Drop streamSubgraphs from the frontend submit paths: subtask progress arrives via root-namespace task_* custom events, so the flag only exposed the leak. * test(runtime): add production-shaped subgraph stream regression tests Address review: the namespace tests validated the publishing helpers with hand-fed namespaces, while the #4399 regression lived in the integration between LangGraph's delegation routing and the worker's stream loop. Add TestWorkerSubgraphStreamIntegration: a real parent graph delegates through the real SubagentExecutor and streams through run_agent into a real MemoryStreamBridge, locking both stream_subgraphs modes -- delegated frames arrive namespaced (never bare), a delegated error fallback cannot mark the parent run as errored, and without the flag delegated frames stay out while task_* custom events remain. |
||
|
MiaoRuidx
|
f1632cc351
|
fix(run): add run event stream contract (#4342)
* docs: document run event stream contract * fix(run): address event stream review feedback --------- Co-authored-by: MiaoRuidx <12540796+MiaoRuidx@users.noreply.github.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
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. |
||
|
Admire
|
a38b1daec3
|
fix(streaming): keep large file generation responsive (#4354)
* fix(streaming): keep large file generation responsive * fix(streaming): address follow-up review feedback * fix(streaming): address final review feedback |
||
|
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>
|
||
|
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. |
||
|
Huixin615
|
65afc9b1d2
|
fix(skills): apply allowed-tools only to active skills (#4098)
* fix(skills): scope allowed-tools to active skills * fix(skills): tolerate stale active skill paths * chore: retrigger CI * fix(skills): document policy activation limits * perf(skills): reuse per-step tool policy decisions * fix(skills): harden runtime tool policy contracts * fix(skills): redact cached policy decisions * fix(skills): make slash tool policy authoritative * fix(skills): preserve policy-safe discovery tools * test(skills): cover explicit task delegation policy |
||
|
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> |
||
|
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 |
||
|
Daoyuan Li
|
45865e9f3f
|
fix(tracing): attach Langfuse trace metadata to the goal evaluator (#4202)
The goal evaluator (runtime/goal.py) runs from runtime/runs/worker.py after
the main graph run has already completed, so there is no graph root for it
to inherit tracing from. create_goal_evaluator_model was built with
attach_tracing=False, and evaluate_goal_completion invoked the model with a
bare config={"run_name": "goal_evaluator"} — no tracing callbacks, no
Langfuse session/user attribution. Every goal-evaluator LLM call went
untraced.
Same class of gap fixed by #2944 for the main agent graph and by #3902 for
memory_agent/suggest_agent: a standalone call site that invokes a model
directly instead of through a traced graph root must attach its own tracing
callbacks and inject Langfuse trace-attribute metadata itself.
- create_goal_evaluator_model: attach_tracing=False -> True, matching the
other standalone non-graph callers (oneshot_llm.run_oneshot_llm,
MemoryUpdater).
- evaluate_goal_completion: accept optional thread_id/user_id/
deerflow_trace_id and inject Langfuse trace metadata onto the ainvoke
config via the shared inject_langfuse_metadata() helper, mirroring
oneshot_llm.py's pattern.
- worker.py: thread user_id (resolve_runtime_user_id(runtime)) and
deerflow_trace_id through _prepare_goal_continuation_input into
evaluate_goal_completion so the evaluator's trace groups under the
triggering run's thread/session.
Updates the existing test that pinned attach_tracing=False as expected
behavior, and adds a regression test asserting the ainvoke config carries
Langfuse trace metadata when enabled.
|
||
|
Daoyuan Li
|
fa419b3e4c
|
fix(goal): stop continuation_count double-bump in thread_changed_before_continuation stand-down (#4199)
_prepare_goal_continuation_input calls the same _persist closure twice with the identical next_count in one evaluation cycle: once to commit the real continuation, and again to record a thread_changed_before_continuation stand-down if a race is detected right after that commit. The second call re-passed continuation_count=next_count, so #4088's defensive max(continuation_count, current_count + 1) guard saw the first call's own write as a "current_count" bump and added another +1 on top of it - silently consuming 2 units of the continuation budget for a cycle that delivered zero actual continuations. #4088's guard is correct for the independent-concurrent-continuations race it targets; this is a separate call site incorrectly re-triggering that same guard against itself. The second _persist call no longer passes continuation_count, matching every other stand-down call site in this function - the count was already correctly committed by the first call. Adds a regression test mirroring the existing thread_changed_after_evaluation race test's checkpointer-wrapper technique, since this sibling branch had zero prior coverage. |
||
|
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 |
||
|
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
|
2fa0505070
|
fix(skills): activate a slash skill once per run, not per model call (#4103)
* fix(skills): activate a slash skill once per run, not per model call SkillActivationMiddleware injects the activation reminder for a slash command via request.override(messages=...), which LangChain's create_agent uses for a single model call and never writes back to graph state. The dedup guard scans request.messages for a prior reminder, but model_node rebuilds request.messages fresh from persisted state on every tool-loop step, so the reminder is never present on the 2nd..Nth model call of a turn. Every model call therefore re-parsed the command, re-read SKILL.md from disk, re-injected the multi-KB body, and re-recorded an "activate" audit event, despite the code intending a single activation per run (#3861 semantics: one activation call, many follow-up model calls). Key the dedup off the run context instead, which LangGraph threads through every model-node call of a run (the same durable signal the request-scoped secret source already uses). The activation call records the slash message's identity in context; later calls for the same message skip re-activation. A new user slash message keys differently and still activates. Secret binding is unaffected: it already re-resolves from the persisted slash source on every call. Adds regression tests that rebuild the real multi-call turn state and assert a single activation across the tool loop, plus a test proving a new slash command still activates. * fix(skills): address review nits on run-scoped activation dedup - Extract _already_activated(run_context, run_key) so the dedup check mirrors the existing _has_existing_activation_for_target sibling instead of an inline dense conditional. - Compute _activation_run_key() once in _find_activation_target and thread it through _prepare_model_request instead of recomputing it at the write site, making the "same key for check and write" invariant explicit in the code rather than implicit. - Document why the run-context write is an overwrite rather than an append/set: only the latest real user message is ever considered an activation target, so there is nothing earlier in the run worth preserving. - Add a regression test locking in the degraded-path contract: when runtime.context is None, the middleware still activates per-call instead of crashing or wrongly no-op'ing. |
||
|
黄云龙
|
224f8de2cf
|
fix(goal): prevent continuation_count regression from racing continuations (#4088)
* fix(goal): prevent continuation_count regression from racing continuations * fix(goal): prevent continuation_count regression from racing continuations |
||
|
黄云龙
|
8be7411da8
|
fix(runtime): serialize SQLite event-store writes to prevent per-thread seq collisions (#4077)
* fix(run-events): serialize seq assignment with a per-thread asyncio lock put() and put_batch() read max(seq) and then INSERT seq+1 in separate awaits. Two coroutines writing the same thread in one process could interleave between the read and the insert and assign the same seq, colliding on SQLite where the DB-level FOR UPDATE lock is weaker than Postgres. Add a per-thread asyncio.Lock (_write_locks / _get_write_lock) around the read-assign-insert critical section in both methods. * address review: evict orphaned per-thread write-lock in delete_by_thread _write_locks accumulated one asyncio.Lock per thread ever seen and never released, leaking in the long-lived DbRunEventStore singleton. Evict the entry after delete_by_thread when no writer holds it (lock recreated lazily on the next write). Per @willem-bd review on #4077. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
黄云龙
|
18c32beaf1
|
fix(runs): re-buffer subagent event batch on flush failure instead of dropping (#4082)
_SubagentEventBuffer.flush() cleared self._pending before the put_batch and discarded the batch when persistence raised, so a transient store error silently lost subagent step events. On failure, prepend the failed batch back onto self._pending (ahead of events queued since) so a later flush can retry it. |
||
|
Daoyuan Li
|
c143c0415b
|
fix(config): Make the sync checkpointer honor the unified database config (#3994)
* Make the sync checkpointer honor the unified database config
The sync checkpointer factory (`get_checkpointer` and `checkpointer_context`)
read only the legacy `checkpointer:` config section and fell back to
`InMemorySaver` when it was absent — it never consulted `database:`. The async
`make_checkpointer` factory and both sync/async Store providers already resolve
the unified `database:` section (legacy `checkpointer:` takes precedence,
otherwise `database:` drives the backend), so the sync checkpointer was the
lone outlier.
Consequence: with `database: {backend: sqlite|postgres}` and no legacy
`checkpointer:` section, the sync checkpointer silently returned `InMemorySaver`
while the Store — same process, same config — correctly persisted to
sqlite/postgres. Embedded callers (`DeerFlowClient`) and the TUI hit this: e.g.
the TUI writes `threads_meta` rows to sqlite (thread appears in the Web UI) but
its checkpoints went to memory and were lost on exit. This also contradicts
backend/AGENTS.md ("the unified `database` section selects the Gateway's
LangGraph checkpointer, LangGraph Store, and DeerFlow SQL repositories").
Mirror the sync Store provider: add `_resolve_checkpointer_config` /
`_get_checkpointer_config` (legacy precedence, else translate `database:` into
a CheckpointerConfig) and route both the singleton and the context manager
through them. The Gateway is unaffected — it uses the async path.
Adds a TestCheckpointerDatabaseConfig suite mirroring the Store's database
tests (singleton + context-manager honor `database:`, legacy precedence,
explicit-memory, missing-config fallback). The two `uses_database_config`
tests fail (return InMemorySaver) before the fix and pass after.
* Handle database=None in sync checkpointer resolution
The unified-config change made `checkpointer_context` / `get_checkpointer`
consult `app_config.database` when no legacy `checkpointer:` section is set.
`AppConfig.database` is always a `DatabaseConfig` in production, but the
existing regression test for issue #1016
(`test_checkpointer_none_fix.py::test_sync_checkpointer_context_returns_in_memory_saver_when_not_configured`)
mocks the app config with `database` left unset, exercising a `database=None`
path that now raised `ValueError: Unknown database backend`.
Mirror the async `make_checkpointer` factory, which already tolerates
`database=None` (falls back to memory), by treating a `None` database as the
memory backend in `_resolve_checkpointer_config`. Update the sync test to set
`mock_config.database = None`, matching its async sibling in the same file.
* Address review: mirror None-guard in store resolver, add sqlite coverage
|
||
|
黄云龙
|
ad9ec65c6f
|
fix(stream_bridge): add stream_exists to MemoryStreamBridge, fixing SSE hang on reconnect after cleanup (#4071)
MemoryStreamBridge is the DEFAULT stream backend. Its subscribe() creates a fresh ended=False stream on every call via _get_or_create_stream(), but the Gateway's reconnect guard _terminal_record_stream_missing only detects a missing stream on bridges that expose stream_exists — and MemoryStreamBridge did NOT define it (only RedisStreamBridge did). After worker cleanup pops the stream (~60s post-run), a browser SSE reconnect or POST /wait hits subscribe() -> creates a zombie stream -> yields heartbeat forever without ever sending END_SENTINEL. The UI spinner never resolves and the coroutine pins a server-side connection/request until external timeout. Add the missing stream_exists method, mirroring RedisStreamBridge. |
||
|
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> |
||
|
Huixin615
|
66c0c641bf
|
fix(gateway): live-tail malformed redis reconnect ids (#4012)
* fix(gateway): live-tail malformed redis reconnect ids * test: avoid redis live-tail heartbeat gaps * fix: handle terminal malformed redis reconnect ids |
||
|
Ryker_Feng
|
26d7a5970d
|
feat: add manual context compaction (#3969)
* feat: add manual context compaction * fix: harden manual context compaction |
||
|
AnoobFeng
|
8fbcdf821b
|
Fix/card tool message bug (#3976)
* fix(runtime): add final reconciliation for missed tool messages * fix(gateway): persist hidden human input card responses Persist allowlisted hidden human_input_response messages in RunJournal so Human Input Cards can recover answered state from run_events after checkpoint compaction. Keep generic internal hidden messages filtered and add regression coverage for ask_clarification responses. |
||
|
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> |
||
|
d33kayyy
|
84bbdf6e00
|
fix(langfuse): resolve trace user from runtime context (#3794)
* fix(langfuse): resolve trace user from runtime context The worker built langfuse_user_id from get_effective_user_id(), which reads the request-scoped _current_user ContextVar. For runs invoked over an internal token on behalf of an end user, that ContextVar is never the end user, so traces recorded langfuse_user_id="default". Switch to resolve_runtime_user_id(runtime), matching the sandbox middleware/tools sites: it reads runtime.context["user_id"] (the owner carried in the run request's context, which survives background-task boundaries) and falls back to get_effective_user_id() for no-auth / browser paths. Caller-supplied metadata still wins via inject_langfuse_metadata's setdefault. * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.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> |
||
|
Zheng Feng
|
dcb2e687d5
|
feat(channels): add GitHub as a webhook-driven channel (#3754)
* feat(channels): add GitHub event-driven agents (#3754) Add a webhook-driven GitHub channel with fail-closed webhook routing, deterministic per-agent PR/issue threads, mention-gated trigger fan-out, GitHub App token injection for sandboxed gh/git commands, and backend/AGENTS.md documentation. * fix(llm-middleware): classify bare IndexError as transient Upstream chat providers occasionally return 200 OK with an empty generations list (observed against Volces "coding" on ark.cn-beijing.volces.com). When that happens, langchain_core.language_models.chat_models.ainvoke raises ``IndexError: list index out of range`` at ``llm_result.generations[0][0].message`` and kills the run. Treat a bare IndexError reaching the middleware as a transient upstream-payload glitch and route it through the existing retry/backoff path instead of failing the whole agent run. The retry budget and backoff schedule are unchanged. Adds three regression tests covering the classifier and both the recover-on-retry and exhausted-retries paths. * fix(runtime): ignore stale LLM fallback markers from prior runs When a run on a thread ends with the LLM-error-handling middleware emitting a `deerflow_error_fallback`-marked AIMessage (e.g. after the IndexError empty-generations classification fix lands), that message is persisted to the thread's checkpoint as part of the messages channel. LangGraph replays the full message history in `stream_mode="values"` chunks, so every subsequent run on the same thread re-streams the stale fallback marker — and the worker's chunk scanner faithfully picks it up, flipping `RunStatus.success` to `RunStatus.error` for runs that themselves had no LLM failure at all. Snapshot the set of pre-existing message ids from the pre-run checkpoint and thread it through `_extract_llm_error_fallback_message` / `_try_extract_from_message` as a filter. Markers on history messages are ignored; markers on fresh messages produced during this run still trip the error path. Falls back to an empty set when the checkpointer is absent or the snapshot can't be captured, preserving the prior behavior on first-run / no-state paths. Adds unit tests for the new filter (helper-level and `_collect_pre_existing_message_ids`) plus an integration test exercising the full `run_agent` path with a stale history checkpointer. * fix(channels): make github channel fire-and-forget to avoid httpx.ReadTimeout on long runs GitHub agent runs (clone -> edit -> test -> push -> PR) routinely exceed the langgraph_sdk default 300s read deadline. The manager's runs.wait call kept an HTTP stream open for the entire run lifetime, so the long run blew up with httpx.ReadTimeout and the outer except branch then released the dedupe key and emitted a false 'internal error' outbound. The GitHub channel's outbound send is log-only by design: agents post to the issue/PR via the gh CLI in the sandbox when they choose to comment or create a PR. There is nothing for the manager to ferry back, so the long-poll was pure overhead. This change adds ChannelRunPolicy.fire_and_forget (default False) and sets it True for the github channel. When fire_and_forget is True, _handle_chat dispatches via client.runs.create (short POST, returns once the run is pending) instead of client.runs.wait, and skips the response-extraction + outbound-publish block. ConflictError on a busy thread still trips the standard THREAD_BUSY_MESSAGE path so behavior on the busy case is preserved for any future non-github fire-and-forget channel. Other (non-github) channels are unchanged: their policy defaults fire_and_forget=False and they continue to dispatch via runs.wait. Adds 6 regression tests in tests/test_channels.py::TestGithubFireAndForget: - Default ChannelRunPolicy.fire_and_forget is False. - The github policy registers fire_and_forget=True. - github inbound calls runs.create, not runs.wait, with the right kwargs. - github inbound publishes no outbound on success. - ConflictError from runs.create still emits THREAD_BUSY_MESSAGE. - Non-github channels (slack) still dispatch via runs.wait. * test(lead-agent): accept user_id kwarg in skill-policy test stubs The two GitHub-channel tests added in #3754 stubbed _load_enabled_skills_for_tool_policy with a lambda that only accepted `available_skills` and `app_config`, but the real function (and its call site in agent.py) also passes `user_id`. This raised TypeError on every run, failing backend-unit-tests. Add `user_id=None` to match the three sibling stubs in the same file. * refactor(gateway): disambiguate context-key set names The two frozensets _INTERNAL_ONLY_CONTEXT_KEYS and _CONTEXT_ONLY_KEYS shared a confusable "CONTEXT_ONLY" token in different orders, and the first broke the _CONTEXT_<X>_KEYS pattern of its sibling _CONTEXT_CONFIGURABLE_KEYS. Rename to make the distinct axes explicit: _CONTEXT_INTERNAL_CALLER_KEYS - WHO: internal callers (scheduler) only _CONTEXT_RUNTIME_ONLY_KEYS - WHERE: runtime context only, never configurable Pure rename, no behavior change. |
||
|
Xinmin Zeng
|
4fc08b4f15
|
feat: add scheduled tasks MVP (#3898)
* feat: add scheduled tasks MVP
* fix: harden scheduled task execution semantics
* feat(scheduled-tasks): preset-driven schedule form with timezone and live preview
Replace the raw cron input with a preset Select (hourly/daily/weekly/monthly/custom)
plus structured inputs (time picker, weekday toggles, day-of-month), datetime-local
for one-time tasks, a timezone selector defaulting to the browser timezone, and a
live human-readable preview. Reuses one ScheduledTaskScheduleInput for create and
edit; backend contract unchanged; zero new deps (pure Intl + DST-safe offset helpers).
* feat(scheduled-tasks): full-page i18n + recipe templates + E2E locale pin
Localize the rest of the scheduled-tasks page (filters, detail pane, actions,
edit form, run list, enum values) via t.scheduledTasks.* in en/zh. Add four
built-in recipe templates (GitHub Trending, news digest, issue triage, weekly
report) exposed as a chip row that pre-fills title + prompt + schedule. Pin
Playwright locale to en-US so E2E selectors stay stable against i18n. No backend
change, no new deps.
* fix(scheduled-tasks): idempotent 0003 migration, update head constants, future-date once test
Merge with main surfaced three CI failures:
- 0003_scheduled_tasks create_table collided with legacy test seeds that
build from full metadata; guard with inspector.has_table so the revision
no-ops when the table already exists (0004/0005 are already idempotent via
_helpers.py).
- persistence bootstrap concurrency/regression tests pinned HEAD to main's
0002_runs_token_usage; bump to the new head 0005_scheduled_task_thread_nullable.
- once-task router test used a fixed past run_at and tripped the
must-be-in-the-future validation; use a future date.
* address review: ok-check, 502 for trigger failure, mock fields, migration filename, doc fences
- fetchThreadScheduledTasks now checks response.ok like the other fetchers.
- trigger endpoint returns 502 (not 409) when dispatch fails outright, so
clients can distinguish a real conflict from a server-side failure.
- E2E mock normalizes scheduled-task objects with context_mode/last_thread_id
and nullable thread_id, matching the backend contract the UI renders against.
- Rename 0002_scheduled_tasks.py -> 0003_scheduled_tasks.py to match its
revision id (file was renamed in spirit already; filename now follows).
- CONFIGURATION.md: close the Tool Groups yaml fence and drop the stray fence
after the Scheduler notes so the sections render correctly.
* fix(scheduled-tasks): harden lease, poller, config, and frontend UX after review
* fix(scheduled-tasks): harden run lifecycle, overlap skip, non_interactive gating, and DST conversion after review
- defer a once task's terminal status to the run-completion hook; the task
stays running until the real outcome, and a startup sweep cancels once
tasks orphaned by a crash (launch-time 'completed' could stick forever)
- record interrupted runs as a distinct 'interrupted' run status with a
readable message; an interrupted once task ends 'cancelled', not 'failed'
- enforce overlap_policy=skip for fresh_thread_per_run via an active-run
pre-check (same-thread ConflictError can never fire across fresh threads)
- protect terminal run statuses from the late launch-path 'running' write
- honor context.non_interactive only for internally-authenticated callers;
arbitrary clients can no longer strip ask_clarification
- fix DST-stale timezone offset in zonedLocalToUtcIso by re-deriving the
offset at the resolved instant (once tasks fired an hour late around
spring-forward and the create->edit round-trip diverged)
- drop dead ScheduledTaskRunRepository.update_by_run_id; share one Gateway
API error helper between channels and scheduled-tasks frontends
* fix(scheduled-tasks): close review round-3 gaps in guards, concurrency, and API ergonomics
- scrub internal-only context keys (non_interactive) from the assembled run
config for non-internal callers: gating body.context alone left the same
key smuggle-able through the free-form body.config copied verbatim by
build_run_config
- guard update_after_launch with protect_terminal so the launch bookkeeping
write cannot clobber a once task already finalized by a fast-failing run's
completion hook (parent-row sibling of the run-row guard)
- reject a manual trigger while the task has an active run (409) instead of
launching a duplicate concurrent run on fresh_thread_per_run
- re-arm a terminal once task to enabled when PATCH pushes run_at into the
future; previously the endpoint returned 200 with a next_run_at that could
never be claimed
- make max_concurrent_runs a real global cap: each poll claims only into the
remaining budget of active (queued/running) scheduled runs
- paginate GET /scheduled-tasks/{id}/runs (limit<=200, offset) and push the
thread filter of /threads/{id}/scheduled-tasks into SQL
- stamp context.user_id on scheduler-launched runs, matching IM channels, so
user-scoped guardrail providers see the owning user
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
|
||
|
Janlay
|
38342b15a3
|
fix(redis): stream retention recovery (#3933)
* fix(gateway): retain redis streams safely * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> |
||
|
Janlay
|
72f033fbbe
|
feat(gateway): add redis stream bridge (#3191)
* feat: add redis stream bridge * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix(gateway): address redis stream bridge review Redis was imported eagerly through deerflow.runtime and declared as a hard dependency, which made memory-only installs load redis.asyncio at startup and left the lazy factory import ineffective. Move redis behind an optional extra, remove the public eager re-export, and keep make_stream_bridge as the only runtime import path with an actionable install hint when the extra is missing. Because Docker deployments now default the stream bridge to Redis via DEER_FLOW_STREAM_BRIDGE_REDIS_URL, install the redis extra explicitly in Docker/dev container flows and teach the local uv-extra detector to infer redis from both stream_bridge.type and the Redis URL env var. This keeps Docker working while preserving slim non-Docker installs. Harden the Redis bridge by batching XREAD replay, replacing brittle ResponseError string matching with a single fallback to 0-0 for malformed Last-Event-ID values, documenting connection/retention/fail-hard behavior, and adding fake plus opt-in real Redis coverage for XADD/XREAD, replay, invalid IDs, and MAXLEN trimming. * fix(config): bump config version for stream bridge * fix redis stream bridge terminal handling * fix: repair uv.lock, format redis.py, and align Dockerfile extras test The uv.lock file was missing a closing bracket for the redis extras section, redis.py had a formatting issue caught by ruff, and the Dockerfile extras test did not account for the hardcoded --extra redis flag. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> |
||
|
DanielWalnut
|
25ea6970a6
|
feat(runtime): implement goal continuations (#3858)
* implement goal continuations * fix(goal): address review findings for goal continuations - goal: key the no-progress breaker on a signature of the latest visible assistant evidence instead of the evaluator's volatile free-text, so it actually fires on stalled turns; thread the signature through every worker persist / no-progress call site - goal: align _stand_down_reason default caps with should_continue_goal (8 / 2) so the two gate functions agree on goals missing the fields - runtime: offload the synchronous checkpointer fallback via asyncio.to_thread (goal.py + worker.py) to keep blocking IO off the loop - frontend: i18n the GoalStatus "Goal" label (goalLabel in en/zh/types) - frontend: extract pure composer helpers into input-box-helpers.ts with unit tests (parseGoalCommand, readGoalResponseError, skill suggestions) - tests: cover the evidence-based no-progress and default-cap behavior - docs: align backend/AGENTS.md goal paragraph with actual behavior - e2e: prettier-format chat.spec.ts (fixes the lint-frontend CI failure) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(frontend): hide goal continuation counter until the agent continues The goal status bar rendered a raw "0/8" before any auto-continuation, which read as a mysterious score. Now the counter is hidden until continuation_count > 0, then shows "Continuing N/M" with a tooltip explaining the auto-continuation cap. - Extract getGoalContinuationDisplay into a pure helper (hides at 0) + unit tests - Add goalContinuing / goalContinuationTooltip i18n keys (en/zh/types) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(goal): address review findings for goal continuations Frontend correctness - Fix the optimistic /goal result permanently shadowing server goal state: the streamed continuation counter never surfaced for a goal set in-session. Extract a shared useActiveGoal hook (used by both chat pages) that reconciles the optimistic copy with server state via a goalReconciliationKey, de-duping the copy-pasted goal block across the two pages. - Stop /goal status|clear failures from escaping handleSubmit as unhandled rejections (handleGoalCommand now returns success; the run only starts when a goal was actually saved). - Use a function replacer for the goal-status toast so an objective containing $&/$1 isn't treated as a replacement pattern. Backend cleanliness / correctness - De-duplicate four byte-identical helpers (_call_checkpointer_method, _message_type, _additional_kwargs, _is_visible_message) by importing them from runtime.goal instead of re-defining them in the run worker. - Remove the dead `checkpoint_tuple.tasks` durability guard (CheckpointTuple has no tasks field) and document that pending_writes is the durability signal. - Decompose the 176-line _prepare_goal_continuation_input: extract _reread_goal_and_checkpoint and a _persist closure so the thread-unchanged guard and stand-down persistence aren't open-coded three times. Document the last-writer-wins write-window limitation as a follow-up. - Add a shared parse_goal_command helper and use it from the TUI and IM-channel /goal handlers (one place for the status/clear/set semantics). Tests - Restore the 11 command-registry tests dropped by the previous goal change (filter_commands ranking/description, build_registry builtins/skills, resolve cases) alongside the new goal tests. - Add coverage for the IM-channel _handle_goal_command, the TUI _handle_goal handler, parse_goal_command, and goalReconciliationKey. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix goal review feedback * fix goal continuation checkpoint races * prioritize goal commands while streaming Route composer submits through a shared helper so /goal commands can be handled before the streaming stop shortcut, while ordinary streaming submits still stop the active run. Testing: cd frontend && pnpm exec rstest run tests/unit/components/workspace/input-box-helpers.test.ts tests/unit/components/workspace/goal-status-helpers.test.ts; cd frontend && pnpm check * preserve goal status during clarification Keep omitted stream goal fields distinct from explicit null clears so clarification interrupts do not hide an active thread goal that is still present in the checkpoint. Testing: pnpm exec rstest run tests/unit/components/workspace/use-active-goal.test.ts tests/unit/components/workspace/input-box-helpers.test.ts tests/unit/components/workspace/goal-status-helpers.test.ts; pnpm check; git diff --check * style: format active goal hook Run Prettier on use-active-goal.ts to satisfy the frontend lint workflow formatting gate. Testing: pnpm format; pnpm exec rstest run tests/unit/components/workspace/use-active-goal.test.ts tests/unit/components/workspace/input-box-helpers.test.ts tests/unit/components/workspace/goal-status-helpers.test.ts; pnpm check; git diff --check * fix goal review followups --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
AnoobFeng
|
e3e5c73b03
|
feat(observability): add trace-id correlation and enhanced logging (#3902)
* feat(observability): add trace-id correlation and enhanced logging - add opt-in gateway request trace correlation via X-Trace-Id - enhance logging with configurable trace_id-aware formatting - propagate deerflow_trace_id into runtime context and Langfuse metadata - keep enhanced logging disabled by default to preserve existing behavior * fix: harden trace correlation wiring - Make logging enhancement a restart-required startup snapshot and remove per-request config reads from TraceMiddleware - Restrict trace ids to printable ASCII before writing them to response headers, logs, and Langfuse metadata - Gate implicit DeerFlowClient trace-id creation behind logging.enhance.enabled while preserving explicit caller opt-in - Bind embedded client trace context per stream step to avoid generator ContextVar leaks and cross-context reset errors - Rebind memory update trace ids in Timer/executor worker paths so enhanced logs keep the captured correlation id - Remove unrelated __run_journal context overwrite from the trace-correlation change set * fix(gateway): avoid eager app construction on package import * fix(gateway): avoid config load during app import Keep Gateway app construction import-safe when config.yaml is absent by disabling TraceMiddleware only for that construction-time fallback path. Startup lifespan still performs strict config loading before serving. |
||
|
Xinmin Zeng
|
09988caf95
|
feat(skills): request-scoped secrets for skills (closes #3861) (#3871)
* feat(sandbox): per-call env injection + platform-secret scrubbing for skills Add an env parameter to Sandbox.execute_command (abstract + local + AIO) so request-scoped secrets can be injected into skill subprocesses, and scrub platform credentials (*KEY*/*SECRET*/*TOKEN*/*PASSWORD*/*CREDENTIAL*) from the inherited environment by default so scoped injection is not security theatre. LocalSandbox always passes an explicit scrubbed env; AioSandbox routes env-bearing commands through bash.exec(env=) on a fresh session and leaves the legacy persistent-shell path unchanged. Part of #3861. BEHAVIOR CHANGE: execute_command no longer inherits the full os.environ; Windows encoding tests updated to assert the scrubbed dict. * feat(skills): parse required-secrets frontmatter declaration Add SecretRequirement and Skill.required_secrets, and parse the required-secrets SKILL.md frontmatter field (a string list or {name, optional} mappings), dropping malformed entries with a warning so one bad declaration does not invalidate the skill. The declared name is both the context.secrets key and the env var injected at activation. Part of #3861. * feat(runtime): request-scoped secret carrier (context.secrets) Add SECRETS_CONTEXT_KEY + extract_request_secrets, centralising the context.secrets carrier contract. The existing context passthrough (build_run_config -> _build_runtime_context) already carries the sub-key to runtime.context without mirroring it into configurable; characterization tests lock that behaviour. Part of #3861. * feat(skills): inject declared secrets at slash-activation into bash env Binding point A: when a skill is slash-activated, SkillActivationMiddleware resolves its declared required-secrets against the request's context.secrets and writes the per-run injection set to runtime.context. The bash tool forwards that set to execute_command(env=). A skill cannot harvest a host platform credential (is_host_platform_secret guard, cf. GHSA-rhgp-j443-p4rf), and injected values are redacted from bash output (mask_secret_values) so an echoed secret never re-enters the prompt/trace. Part of #3861. * test(skills): lock the five secret leak surfaces + add trace redaction helper Regression tests assert the secret value is absent from all five surfaces: prompt (activation message), checkpoint (graph state vs context separation), audit (journal records names only), trace (metadata builder never copies context; never mirrored to configurable), and stdout (mask_secret_values). Add redact_secret_context_keys as a defensive helper for any context serialization. Part of #3861. * docs(backend): document request-scoped secrets for skills Add Request-Scoped Secrets subsection (Skills) + env policy note (Sandbox) and the execute_command(env=) signature change, per the doc-sync policy. Part of #3861. * fix(skills): close gaps found by end-to-end verification of request-scoped secrets Real-gateway e2e + independent review of #3861 surfaced three defects, now fixed: 1. Slash activation never fired in the live chain. InputSanitizationMiddleware wraps user input in BEGIN/END markers before SkillActivationMiddleware sees it, and the original text was only preserved when an upload or IM channel set it. For a plain text message the slash command became undetectable, so no secret was ever resolved. Fix: the sanitizer now setdefaults the pre-wrap text into ORIGINAL_USER_CONTENT_KEY (additive; sanitization behaviour unchanged), so slash activation works for all messages. Pre-existing latent bug surfaced here. 2. The raw request config (with context.secrets) was persisted to runs.kwargs_json and echoed by the run API (RunResponse.kwargs). Fix: redact_config_secrets() strips secret-bearing context keys from the persisted/echoed copy in start_run; the live config that drives the run keeps them. build_run_config now also sets configurable.thread_id on the context path (the checkpointer requires it). 3. Connection-string credentials (DATABASE_URL, REDIS_URL, SENTRY_DSN, GH_PAT, ...) were not scrubbed from the inherited sandbox env. Fix: env_policy adds a *DSN* pattern plus an explicit connection-string denylist (no blanket *URL* — benign service URLs stay readable). Verified end-to-end via a real gateway run (real LLM + skill activation + bash): the secret reaches the sandbox subprocess and appears in NONE of prompt, trace, checkpoint, audit, stdout, runs.kwargs_json, or the run API. Part of #3861. * docs(backend): document the env scrub, persistence redaction, and sanitizer interaction Sync the Request-Scoped Secrets section with the verification-driven fixes: inherited-env scrub (incl. connection-string denylist), run-record/run-API redaction as the 6th sealed leak surface, and the sanitizer preserving original content so slash activation fires. Part of #3861. * fix(skills): inject caller secret over scrubbed host value; drop redundant host-name guard A real-world demo (a skill calling a third-party cloud API with a request-scoped key) exposed that the is_host_platform_secret guard was both wrong and harmful: it refused to inject a caller-supplied secret whenever a same-named variable existed in the Gateway env — which is exactly the #3861 use case (a per-user key overriding a shared platform key). The guard was also redundant: build_sandbox_env already scrubs secret-looking names from the inherited env before injection, so a skill can never read a host credential — it only ever receives the caller's value. Remove the guard; the injected (caller) value simply wins over the scrubbed host value. Verified end-to-end: the agent called the real cloud API successfully with the caller's key, the host's same-named key was scrubbed and never used, and the caller's key leaked to none of the surfaces. Part of #3861. * fix(skills): address review on request-scoped secrets (#3861) Review fixes from PR #3871: - E2BSandbox.execute_command now accepts env/timeout and routes them to commands.run(envs=, timeout=). The bash tool passes env= unconditionally, so the prior signature (command only) raised TypeError on every e2b bash call and broke e2b deployments entirely. env=None stays backward-compatible. - SkillActivationMiddleware clears the active-secret set before resolving each activation, so a later skill in the same run never inherits an earlier skill's injection set (the #3861 contract: a skill only receives what the caller supplied AND that skill declared). - AioSandbox env path uses a dedicated _DEFAULT_HARD_TIMEOUT — bash.exec exposes no idle/no-change timeout, so the prior reuse of the legacy idle constant conflated wall-clock vs idle semantics. The env path also retries on the ErrorObservation signature now, sharing the legacy persistent-shell recovery contract. - mask_secret_values skips values below a minimum length floor so a short declared secret (e.g. "42") cannot shred unrelated bytes (exit codes, timestamps, sizes) of tool output. The secret is still injected into the subprocess; only the output mask skips it. session_id reuse on the env path is intentionally NOT added: a shared session could let request-scoped secrets ride the session env into later commands, which the SDK does not contractually forbid. The fresh-session choice matches the LocalSandbox model (each call is a fresh subprocess); the trade-off (consecutive env-bearing calls do not share cwd/venv/exports) is documented on _execute_with_env. |