* 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>
* 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
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>
* 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.
* 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.
* 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.
* 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>
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.
* feat(context): record effective memory identity per run
* fix(context): address memory identity review feedback
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* 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
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.
_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.
* 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>
* 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
* 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>
* 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>
* 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.
* fix(goal): prevent continuation_count regression from racing continuations
* fix(goal): prevent continuation_count regression from racing continuations
* 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>
_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.
* 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
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.
* 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>
* 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.
* 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>
* 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>
* 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>
* 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>
* 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.
* 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>
* 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>
* 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>
