* feat(persistence): support custom postgres schema
* fix(persistence): address CI lint/test failures and review feedback
- Map missing psycopg import to actionable POSTGRES_INSTALL guidance in
sync/async schema-creation helpers
- Accept SQLAlchemy compound DSN schemes (postgresql+asyncpg) when
injecting search_path, normalizing to a libpq-consumable DSN
- Guard keyword-DSN tests with importorskip so they skip without psycopg
- Set database=None in sync checkpointer none-fix test to avoid MagicMock
backend resolution
- Apply ruff import sort and format
* fix(persistence): address pg-schema review feedback
- Restrict postgres_schema regex to lowercase-only so the quoted CREATE
SCHEMA matches the unquoted search_path (PG case-folds it), fixing the
mixed-case bug where tables silently fell back to public.
- Replace shlex.join/split with libpq-correct backslash escaping for the
options parameter so values containing spaces survive intact.
- Add normalize_libpq_dsn() and route the async checkpointer pool through
dsn_with_search_path() so a +asyncpg suffix is stripped and existing DSN
options (e.g. statement_timeout) are merged instead of overridden.
- Extract shared ensure_postgres_schema()/ensure_postgres_schema_async()
helpers (mapping missing psycopg to the install hint) used by all four
provider sites.
- Tests: reject mixed-case schemas, preserve space-containing libpq option,
cover normalize_libpq_dsn, and assert pool search_path via DSN.
* fix(persistence): align pg-schema test with merged store API
The main merge moved the sync Store factory to the single-path
_resolve_store_config/_sync_store_cm design, dropping the PR's
_sync_store_from_database helper. The integration test still imported
the removed symbol, breaking test collection (backend-unit-tests).
Resolve the store config from a DatabaseConfig and drive it through
_sync_store_cm instead.
* fix(persistence): address pg-schema review feedback
- reject trailing/leading whitespace in postgres_schema via re.fullmatch
(a $-anchored re.match let "deerflow\n" through, silently landing tables
in public)
- re-escape all whitespace (TAB/CR/LF) when re-joining libpq options so a
caller's pre-existing options value round-trips losslessly
- re-validate the identifier inside create_schema_sql as defense-in-depth
at the SQL-emitting boundary
- accept the postgres:// short scheme in the alembic search_path injection
- close the sync psycopg connection explicitly (psycopg3 __exit__ does not
close()), mirroring the async path
- drop the partial checkpointer/store reset on a database config change;
database is restart-required and the ORM engine is not rebuilt, so a
partial reset would half-migrate the deployment
* docs(config): complete the postgres_schema migration checklist
Address PR review (P1): the documented `public`->schema migration only
moved runs, run_events, threads_meta, feedback, and users. That strands
every other DeerFlow-owned table -- the four channel_* tables, both
scheduled_* tables, agents, and (critically) alembic_version -- in
`public`. On restart bootstrap treats the partially-populated target
schema as unversioned, re-baselines it, and replays migrations while the
real rows stay invisible in `public`.
List the full owned set explicitly, call out alembic_version as required,
and keep the "discover the rest" query for version-drift safety.
* refactor(checkpointer): drop test-only _sync_checkpointer_from_database
Address PR review: the helper was only reached by the env-gated
integration test and re-implemented the DatabaseConfig->CheckpointerConfig
backend resolution that _resolve_checkpointer_config already owns, so a
future backend added there would silently miss this path. Mirror the store
side of the same test, which reuses the production path directly:
_resolve_checkpointer_config(...) + _sync_checkpointer_cm(...).
* feat(checkpoint): make delta snapshot_frequency configurable
* fix(config): carry legacy checkpoint_delta_snapshot_frequency with warning
Addresses review on #4516: the rename from the flat
database.checkpoint_delta_snapshot_frequency key to nested
database.checkpoint_delta.snapshot_frequency silently dropped the old
value (pydantic extra="ignore"). Add a before-validator that maps the
legacy key onto the nested one with a deprecation warning (nested key
wins when both are set), plus a CHANGELOG breaking-change note covering
the rename and the 1000 -> 10 default change.
* fix(checkpoint): validate frozen snapshot frequency
* 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>