* docs(specs): read-before-write gate design for issue #3857 output layer
* docs(plans): read-before-write gate implementation plan (#3857)
* refactor(sandbox): extract read_current_file_content helper (#3857)
* feat(middlewares): read-before-write version gate for file tools (#3857)
* test(middlewares): pin async read-before-write gate paths (#3857)
* feat(config): wire ReadBeforeWriteMiddleware into runtime chain, default on (#3857)
* docs(sandbox): document read-before-write gate in tool docstrings and AGENTS.md (#3857)
* docs(plans): align plan doc with landed config_version (17) and drop machine-specific paths
Addresses Copilot review comments on #3912.
* fix(middlewares): read-before-write gate — error-string sandboxes fail open; serialize gate+execution per path (#3912 review)
- AIO/E2B read_file reports failures (incl. missing files) as 'Error: ...'
strings instead of raising; the gate treated that string as existing file
content and blocked first-write creation. Error-string reads now count as
uninspectable: gate fails open, no mark is stamped.
- LangGraph runs one AIMessage's tool calls concurrently, so two same-turn
writes could both pass on one stale mark before either mutation landed
(and a read mark could hash a version the model never saw). Gate check +
tool execution (and read + mark stamping) now share a per-(thread, path)
critical section, separate from the tool-internal file_operation_lock.
* feat: record guardrail decisions in run events
Persist security-relevant GuardrailMiddleware outcomes (deny and
provider-error fail-open/fail-closed) as middleware:guardrail run
events via RunJournal.record_middleware(), mirroring
SafetyFinishReasonMiddleware. Recording is best-effort: it reads
__run_journal from the runtime context and swallows persistence
failures, so tool execution behavior is unchanged.
The audit payload records tool name/id, agent id, subagent flag, user
role, allow decision, policy id, reason codes/messages, fail_closed
mode, and provider_error flag. Tool input/args and identity fields
(user_id, oauth_*) are deliberately excluded to avoid persisting the
sensitive content being blocked.
The fail-closed provider-error branch returns the denied message
directly from the except block so it records exactly once and does
not fall through to the generic deny branch.
* docs: clarify guardrail journal runtime boundary
* fix: preserve subagent attribution in guardrail events
* test: align guardrail event attribution fixtures
* refactor(guardrails): resolve runtime context once per tool call
Extract `_resolve_context` and thread the already-resolved context dict
through `_build_request` and `_record_guardrail_event` so the
getattr/runtime.context chain runs once per wrap_tool_call instead of twice.
Also document the `is_subagent` field boundary: native subagents do not
inherit __run_journal, so the field is structurally False in persisted
records today; custom runtimes may still supply it with attribution.
Addresses review feedback on #3837 (context-read-twice cleanup and the
is_subagent trade-off note). No behavior change.
* feat(mcp): add per-server tool_call_timeout for MCP tool calls
Add a configurable timeout for individual MCP tool calls to prevent
agent runs from blocking indefinitely when an MCP server becomes
unresponsive (e.g., rate-limited HTTP API, hung subprocess).
Uses the MCP SDK's built-in read_timeout_seconds parameter on
ClientSession.call_tool, which handles the timeout within the
session's own task — avoiding cross-task cancellation issues with
the session pool (ref #3379, #3203).
Config field is named tool_call_timeout (not timeout) to avoid
collision with langchain-mcp-adapters' existing timeout field on
HTTP/SSE connections.
Closes#3840
* fix(mcp): read tool_call_timeout from McpServerConfig, not connection dict
The previous implementation put tool_call_timeout into the connection dict
returned by build_server_params, which langchain's create_session then
passed to _create_stdio_session(), causing TypeError. Now reads the timeout
directly from ExtensionsConfig.mcp_servers where the wrapper is built,
keeping it out of the connection dict entirely.
Fixes P1 bug from review on #3843.
* test(mcp): regression test for tool_call_timeout not leaking into connection dict
Adds two tests:
- test_build_server_params_excludes_tool_call_timeout: verifies the connection
dict returned by build_server_params() does NOT contain tool_call_timeout
- test_stdio_tool_call_timeout_does_not_raise_typeerror: end-to-end test that
get_mcp_tools() with a stdio server having tool_call_timeout configured loads
tools without TypeError from _create_stdio_session()
Regression for PR #3843 P1 bug.
* fix(mcp): only pass read_timeout_seconds when tool_call_timeout is set
When tool_call_timeout is None, don't pass read_timeout_seconds=None to
session.call_tool(). This avoids breaking existing tests that assert on
exact call_tool arguments without the extra kwarg.
* docs(mcp): clarify stdio tool timeout
* fix(sandbox): normalize Windows backslash paths to forward slashes in bash commands
On Windows, `replace_virtual_paths_in_command()` and
`LocalSandbox._resolve_paths_in_command()` resolve virtual paths
(/mnt/skills, /mnt/user-data, /mnt/acp-workspace, custom mounts) to
host paths with backslashes (C:\Users\admin\...). Bash interprets
\U, \a, \d, \s, \n, \t as escape sequences, mangling the path.
Fix: add `.replace("\\", "/")` to all path resolution callbacks so
resolved paths use forward slashes, which bash handles correctly on
all platforms and Windows APIs/Python's open() accept natively.
Fixes#3865
* test(sandbox): regression tests for Windows backslash path normalization
Covers replace_virtual_paths_in_command and LocalSandbox._resolve_paths_in_command
with Windows-style backslash paths, asserting no backslashes survive in output.
Closes#3865
* test(sandbox): fix ACP test failure, keep 6 passing regression tests
Removed test_acp_workspace_no_backslash which triggered path traversal
validation. Remaining 6 tests cover user-data (3), skills (1), and
LocalSandbox custom-mount (2) paths — all with Windows backslash paths.
* test(sandbox): restore ACP Windows path normalization coverage
* 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>
* feat(observability): add trace-id correlation and enhanced logging
- add opt-in gateway request trace correlation via X-Trace-Id
- enhance logging with configurable trace_id-aware formatting
- propagate deerflow_trace_id into runtime context and Langfuse metadata
- keep enhanced logging disabled by default to preserve existing behavior
* fix: harden trace correlation wiring
- Make logging enhancement a restart-required startup snapshot and remove per-request config reads from TraceMiddleware
- Restrict trace ids to printable ASCII before writing them to response headers, logs, and Langfuse metadata
- Gate implicit DeerFlowClient trace-id creation behind logging.enhance.enabled while preserving explicit caller opt-in
- Bind embedded client trace context per stream step to avoid generator ContextVar leaks and cross-context reset errors
- Rebind memory update trace ids in Timer/executor worker paths so enhanced logs keep the captured correlation id
- Remove unrelated __run_journal context overwrite from the trace-correlation change set
* fix(gateway): avoid eager app construction on package import
* fix(gateway): avoid config load during app import
Keep Gateway app construction import-safe when config.yaml is absent by
disabling TraceMiddleware only for that construction-time fallback path.
Startup lifespan still performs strict config loading before serving.
* feat(sandbox): per-call env injection + platform-secret scrubbing for skills
Add an env parameter to Sandbox.execute_command (abstract + local + AIO) so request-scoped secrets can be injected into skill subprocesses, and scrub platform credentials (*KEY*/*SECRET*/*TOKEN*/*PASSWORD*/*CREDENTIAL*) from the inherited environment by default so scoped injection is not security theatre. LocalSandbox always passes an explicit scrubbed env; AioSandbox routes env-bearing commands through bash.exec(env=) on a fresh session and leaves the legacy persistent-shell path unchanged. Part of #3861. BEHAVIOR CHANGE: execute_command no longer inherits the full os.environ; Windows encoding tests updated to assert the scrubbed dict.
* feat(skills): parse required-secrets frontmatter declaration
Add SecretRequirement and Skill.required_secrets, and parse the required-secrets SKILL.md frontmatter field (a string list or {name, optional} mappings), dropping malformed entries with a warning so one bad declaration does not invalidate the skill. The declared name is both the context.secrets key and the env var injected at activation. Part of #3861.
* feat(runtime): request-scoped secret carrier (context.secrets)
Add SECRETS_CONTEXT_KEY + extract_request_secrets, centralising the context.secrets carrier contract. The existing context passthrough (build_run_config -> _build_runtime_context) already carries the sub-key to runtime.context without mirroring it into configurable; characterization tests lock that behaviour. Part of #3861.
* feat(skills): inject declared secrets at slash-activation into bash env
Binding point A: when a skill is slash-activated, SkillActivationMiddleware resolves its declared required-secrets against the request's context.secrets and writes the per-run injection set to runtime.context. The bash tool forwards that set to execute_command(env=). A skill cannot harvest a host platform credential (is_host_platform_secret guard, cf. GHSA-rhgp-j443-p4rf), and injected values are redacted from bash output (mask_secret_values) so an echoed secret never re-enters the prompt/trace. Part of #3861.
* test(skills): lock the five secret leak surfaces + add trace redaction helper
Regression tests assert the secret value is absent from all five surfaces: prompt (activation message), checkpoint (graph state vs context separation), audit (journal records names only), trace (metadata builder never copies context; never mirrored to configurable), and stdout (mask_secret_values). Add redact_secret_context_keys as a defensive helper for any context serialization. Part of #3861.
* docs(backend): document request-scoped secrets for skills
Add Request-Scoped Secrets subsection (Skills) + env policy note (Sandbox) and the execute_command(env=) signature change, per the doc-sync policy. Part of #3861.
* fix(skills): close gaps found by end-to-end verification of request-scoped secrets
Real-gateway e2e + independent review of #3861 surfaced three defects, now fixed:
1. Slash activation never fired in the live chain. InputSanitizationMiddleware
wraps user input in BEGIN/END markers before SkillActivationMiddleware sees it,
and the original text was only preserved when an upload or IM channel set it.
For a plain text message the slash command became undetectable, so no secret
was ever resolved. Fix: the sanitizer now setdefaults the pre-wrap text into
ORIGINAL_USER_CONTENT_KEY (additive; sanitization behaviour unchanged), so
slash activation works for all messages. Pre-existing latent bug surfaced here.
2. The raw request config (with context.secrets) was persisted to runs.kwargs_json
and echoed by the run API (RunResponse.kwargs). Fix: redact_config_secrets()
strips secret-bearing context keys from the persisted/echoed copy in start_run;
the live config that drives the run keeps them. build_run_config now also sets
configurable.thread_id on the context path (the checkpointer requires it).
3. Connection-string credentials (DATABASE_URL, REDIS_URL, SENTRY_DSN, GH_PAT, ...)
were not scrubbed from the inherited sandbox env. Fix: env_policy adds a *DSN*
pattern plus an explicit connection-string denylist (no blanket *URL* — benign
service URLs stay readable).
Verified end-to-end via a real gateway run (real LLM + skill activation + bash):
the secret reaches the sandbox subprocess and appears in NONE of prompt, trace,
checkpoint, audit, stdout, runs.kwargs_json, or the run API. Part of #3861.
* docs(backend): document the env scrub, persistence redaction, and sanitizer interaction
Sync the Request-Scoped Secrets section with the verification-driven fixes: inherited-env scrub (incl. connection-string denylist), run-record/run-API redaction as the 6th sealed leak surface, and the sanitizer preserving original content so slash activation fires. Part of #3861.
* fix(skills): inject caller secret over scrubbed host value; drop redundant host-name guard
A real-world demo (a skill calling a third-party cloud API with a request-scoped
key) exposed that the is_host_platform_secret guard was both wrong and harmful:
it refused to inject a caller-supplied secret whenever a same-named variable
existed in the Gateway env — which is exactly the #3861 use case (a per-user key
overriding a shared platform key). The guard was also redundant: build_sandbox_env
already scrubs secret-looking names from the inherited env before injection, so a
skill can never read a host credential — it only ever receives the caller's value.
Remove the guard; the injected (caller) value simply wins over the scrubbed host
value. Verified end-to-end: the agent called the real cloud API successfully with
the caller's key, the host's same-named key was scrubbed and never used, and the
caller's key leaked to none of the surfaces. Part of #3861.
* fix(skills): address review on request-scoped secrets (#3861)
Review fixes from PR #3871:
- E2BSandbox.execute_command now accepts env/timeout and routes them to
commands.run(envs=, timeout=). The bash tool passes env= unconditionally,
so the prior signature (command only) raised TypeError on every e2b bash
call and broke e2b deployments entirely. env=None stays backward-compatible.
- SkillActivationMiddleware clears the active-secret set before resolving each
activation, so a later skill in the same run never inherits an earlier
skill's injection set (the #3861 contract: a skill only receives what the
caller supplied AND that skill declared).
- AioSandbox env path uses a dedicated _DEFAULT_HARD_TIMEOUT — bash.exec exposes
no idle/no-change timeout, so the prior reuse of the legacy idle constant
conflated wall-clock vs idle semantics. The env path also retries on the
ErrorObservation signature now, sharing the legacy persistent-shell recovery
contract.
- mask_secret_values skips values below a minimum length floor so a short
declared secret (e.g. "42") cannot shred unrelated bytes (exit codes,
timestamps, sizes) of tool output. The secret is still injected into the
subprocess; only the output mask skips it.
session_id reuse on the env path is intentionally NOT added: a shared session
could let request-scoped secrets ride the session env into later commands,
which the SDK does not contractually forbid. The fresh-session choice matches
the LocalSandbox model (each call is a fresh subprocess); the trade-off
(consecutive env-bearing calls do not share cwd/venv/exports) is documented on
_execute_with_env.
build_run_config() copied every top-level request key (except
configurable/context) verbatim into the LangGraph RunnableConfig, including
recursion_limit. The server default of 100 was fully overridable by the caller
with no upper bound, so a request like {"config": {"recursion_limit": 100000000}}
could make a single run execute effectively unbounded LangGraph super-steps
(each >= 1 LLM call), enabling runaway API cost / DoS.
Validate the client value server-side and clamp it into a safe range:
- valid positive ints are capped at a configurable ceiling
(AppConfig.max_recursion_limit, default 1000 to match the existing
frontend/public-skill default so legitimate deep runs are unaffected)
- invalid/non-positive/bool/None values fall back to the 100 server default
- applied on both the configurable and the LangGraph >= 0.6.0 context paths
- WARNING logged on clamp for observability
Add unit tests (including a configurable-ceiling case), expose
max_recursion_limit in config.example.yaml (config_version 16), and document
the ceiling/fallback in backend/docs/API.md.
Co-authored-by: DengY11 <DengY11@users.noreply.github.com>
* feat(gateway): add GET /api/features for frontend feature gating (#3757)
* feat(agents): add useAgentsApiEnabled feature-flag hook (#3757)
* feat(agents): gate /workspace/agents segment on agents_api flag (#3757)
* feat(sidebar): grey out Agents button with tooltip when agents_api disabled (#3757)
* fix(sidebar): show disabled Agents tooltip on hover, harden a11y + e2e (#3757)
- Wrap the disabled Agents button in a hoverable tooltip trigger so the
'feature not enabled' hint shows in the expanded sidebar, not only when
collapsed (the SidebarMenuButton tooltip prop is hidden unless collapsed).
- Add tabIndex={-1} and drop the redundant onClick preventDefault.
- Add e2e coverage for the disabled state + a default /api/features mock.
* style(sidebar): move cursor-not-allowed to the hoverable span (#3757)
* test(agents): anchor e2e agents-API request filter with a path regex (#3757)
* fix(agents): don't expose backend config in disabled message; tell user to contact admin (#3757)
The disabled panel previously said 'Set agents_api.enabled: true in
config.yaml', leaking backend configuration to end users. Replace with a
generic 'not enabled on this server, contact your administrator' message
(matching the existing nameStepApiDisabledError copy). e2e now asserts the
contact-admin message and that no config.yaml/agents_api text is rendered.
* make format
* fix(agents): keep agents_api flag sticky during /api/features outage (#3757)
Failing open re-mounted the agents UI and re-triggered the 403 storm when
agents_api was genuinely disabled and /api/features was down. Persist the
last definitive answer and fall back to it (sticky) before failing open,
only failing open when nothing has ever been observed. Read the cached
value after mount so the first client render matches the server (no
hydration mismatch on the non-loading-gated sidebar).
* fix(sidebar): make disabled Agents entry keyboard/SR accessible (#3757)
The disabled-state explanation was hover-only: tabIndex={-1} removed the
entry from the tab order and the reason lived only in a pointer-triggered
tooltip. Keep it in the tab order and wire aria-describedby to a
visually-hidden reason so keyboard and screen-reader users learn why it is
disabled.
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* feat(community): add Crawl4AI web_fetch provider
Crawl4AI is a self-hosted, no-API-key web fetcher: it runs headless
Chromium and returns server-cleaned "fit" markdown directly via its
POST /md endpoint, so no client-side readability extraction is needed.
It sits alongside the existing self-hosted Browserless provider.
- deerflow.community.crawl4ai: async Crawl4AiClient + web_fetch_tool
(reads base_url/timeout_s/token/filter from config; "Error:" string
convention; 4096-char cap), mirroring the browserless provider
- tests: 17 unit cases (success, HTTP error, success:false, empty,
timeout, request error, token header, truncation, config reads)
- config.example.yaml: commented web_fetch example
- doctor: register as a no-key (free) web_fetch provider
- setup wizard: add to WEB_FETCH_PROVIDERS (no API key)
- docs: README, CONTRIBUTING, CONFIGURATION, AGENTS provider lists
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(community): address Crawl4AI provider review feedback
- timeout: robust _coerce_timeout (bool / non-numeric -> default) mirroring
jina, so 'timeout: off' no longer becomes 0.0 and times out every request
- read web_fetch config once per invocation and pass values into the client,
so a concurrent hot-reload can't split base_url from filter
- rename config key timeout_s -> timeout to match jina/infoquest (the
default providers); update config.example.yaml + setup wizard
- validate + normalize the markdown filter against {fit,raw,bm25,llm};
unknown values fall back to fit with a warning instead of an opaque HTTP 400
- client: a non-JSON 200 body (reverse proxy / auth wall) now reports the
content-type + snippet instead of a generic JSONDecodeError
- tests: 22 cases (added non-JSON-200, _coerce_timeout, _coerce_filter,
invalid-filter fallback, read-config-once)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: DanielWalnut <45447813+hetaoBackend@users.noreply.github.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* fix(gateway): require admin for global skills management endpoints
The skills router exposed its management endpoints with no authorization
check (only Depends(get_config)), while the MCP router guards the
equivalent global extensions_config mutations with require_admin_user
(added in the #3425 security hardening). Skills storage is global/shared
across users (no per-user path), lives in the same extensions_config.json
as MCP servers, and custom skill SKILL.md content is injected into every
user's agent system prompt. Any authenticated non-admin user could mutate
or read global skills that affect all tenants:
- POST /api/skills/install: install from an arbitrary thread_id, writing
into the global custom skills tree
- PUT /api/skills/custom/{skill_name}: rewrite a global skill, injecting
instructions into all users' agent prompts (cross-tenant prompt injection)
- DELETE /api/skills/custom/{skill_name} and rollback: tamper with global skills
- GET /api/skills/custom, GET .../{skill_name}, GET .../history: read raw
global custom skill bodies/history
- PUT /api/skills/{skill_name} (enable toggle): writes the shared
extensions_config.json and refreshes the system prompt for every tenant,
so a non-admin could enable/disable any skill globally. There is no
per-user skill state, so this is a global mutation, not a preference.
Add require_admin_user to every endpoint above, mirroring the MCP router.
The shared read path used internally by update/rollback was extracted into
a non-auth helper (_read_custom_skill_response) so internal reuse does not
double-check auth.
Only the read-only GET /api/skills and GET /api/skills/{skill_name} stay
open to normal users: they return just name/description/enabled and back
the user-facing settings UI.
Tests:
- New tests/test_skills_router_authz.py: a non-admin user gets 403 on every
guarded endpoint (including the enable toggle); basic listing stays open;
admins can still toggle.
- Update tests/test_skills_custom_router.py to authenticate as admin.
- pytest tests/ -k skill -> 350 passed, 1 skipped; ruff clean.
Signed-off-by: DengY11 <151997860+DengY11@users.noreply.github.com>
* fix(frontend): gate skill enable/install UI behind admin + handle 403
Follow-up to the backend change that made the global skills mutations
admin-only. Without a matching UI change, a non-admin user would hit a
silent 403 when toggling a skill in Settings -> Skills or clicking
"Install skill" on a .skill artifact.
- core/skills/api.ts: add SkillRequestError (status + isAdminRequired),
throw it from loadSkills/enableSkill on non-ok responses and from
installSkill on 403 (other install errors keep the soft-failure
contract).
- core/skills/hooks.ts: useSkills no longer retries on SkillRequestError.
- skill-settings-page.tsx: show an "admin required" message on 403, and
disable the enable toggle for non-admins (mirrors the MCP tools page).
- artifact-file-detail.tsx / artifact-file-list.tsx: only render the
"Install skill" action for admins, and surface an admin-required toast
if a 403 still occurs.
- i18n: add settings.skills.adminRequired / installAdminRequired (en + zh).
Auth/no-auth and static-website modes synthesize an admin user, so these
gates do not affect single-user/local deployments.
Verified locally: pnpm check (eslint + tsc) passes with no new errors,
pnpm build succeeds, and the dev server renders / and /login (200) with
no compile/runtime errors.
Signed-off-by: DengY11 <151997860+DengY11@users.noreply.github.com>
* style(frontend): format skills hook for prettier
Keep the admin-guard skills hook aligned with Prettier output so the frontend format check passes in CI.
---------
Signed-off-by: DengY11 <151997860+DengY11@users.noreply.github.com>
* Add Brave image search community tool
* fix(community): length-cap Brave web_search queries
Apply _clean_query in web_search_tool so over-long queries are trimmed
to Brave's 400-char limit before the API call, matching image_search_tool
and avoiding HTTP 422 from the Brave Search API.
* fix(community): harden Brave image search SSRF guard and dimension mapping
Address PR review findings:
- Catch ValueError from urlparse so a malformed bracketed-IPv6 URL skips
one item instead of crashing the whole image_search call
- Reject IPv6 literals embedding a non-global IPv4 (IPv4-mapped, 6to4,
NAT64, IPv4-compatible), closing the loopback/private SSRF bypass
- Report width/height from the dict of the URL actually returned, so a
surviving thumbnail no longer reports the dropped original's dimensions
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
The csrf_token double-submit cookie is set without max_age (a session
cookie), while the access_token cookie is persistent over HTTPS
(max_age = token_expiry_days, see _set_session_cookie). The two cookies
represent one session but had different lifetimes.
iOS Safari home-screen PWAs evict session cookies when iOS terminates the
standalone web app, but keep persistent ones. On reopen the user still
appears logged in (the persistent access_token survives and GET
/api/v1/auth/me is CSRF-exempt), but the session-only csrf_token is gone,
so the frontend's readCsrfCookie() returns null and sends no X-CSRF-Token
header. The first state-changing request then fails with 403 "CSRF token
missing. Include X-CSRF-Token header." Only re-login restored it. This
only manifests over HTTPS, which is why plain-HTTP local dev never sees it.
Give csrf_token the same max_age as access_token at both mint sites --
CSRFMiddleware (auth POSTs) and _set_csrf_cookie (OIDC GET callback) --
mirroring _set_session_cookie's `... if is_https else None` guard so the
double-submit pair always shares a lifetime: persistent together over
HTTPS, session-only together over plain HTTP.
Regression tests in tests/test_auth_type_system.py:
- test_csrf_cookie_persistent_on_https
- test_csrf_cookie_session_only_on_http
- test_oidc_callback_csrf_cookie_persistent_on_https
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
extract_outline appended the {truncated: True} sentinel as soon as len(outline) >= MAX_OUTLINE_ENTRIES, i.e. right after the 50th heading, before knowing whether a 51st exists. A document with exactly 50 headings was therefore reported as truncated, and uploads_middleware injected a misleading 'showing first 50 headings' hint into the agent's context. Use a lookahead: only mark truncated once a heading beyond the limit is actually seen, then drop that extra entry. Adds an exact-boundary regression test.
* feat(subagents): persist and display subagent step history (#3779)
Capture both assistant turns and tool outputs during subagent execution,
stream them in task_running events, and persist them as subagent.* run
events so the subtask card's step timeline survives a reload.
Backend:
- step_events.py: pure layer (capture_step_message, build_subagent_step,
subagent_run_event) shared by streaming and persistence
- executor.py: capture ToolMessage outputs, not just AIMessage turns
- worker.py: persist task_* custom events to RunEventStore (category
"subagent" keeps them out of the thread feed; list_events backfills)
Frontend:
- core/tasks/steps.ts + api.ts: SubtaskStep model, messageToStep,
eventsToSteps, mergeSteps, fetchSubtaskSteps
- subtask card accumulates live steps and backfills on expand
- carry run_id onto history content messages for the events endpoint
* fix(subagents): show AI turns in subtask card + paginate step backfill (#3779)
Two follow-ups to the subagent step-history feature:
Problem 1 — reload backfill could silently truncate the step timeline because
list_events capped at 500 events (seq-ASC) across the whole run. Add task_id
filtering + an after_seq forward cursor to list_events (all three stores +
abstract base + the /events route), and make fetchSubtaskSteps page through one
task's subagent.step events until a short page. No schema migration: the DB
filter rides the existing run-scoped index via event_metadata["task_id"].
Problem 2 — the card only rendered tool steps, so persisted AI turns were never
shown. Replace toolStepsForDisplay with stepsForDisplay: interleave AI reasoning
turns (with text) and tool steps by message_index, drop blank-text AI turns, and
drop the trailing final-answer AI turn when completed (already shown as result).
Card renders AI steps as muted clamped markdown with a sparkles icon.
Tests: store task_id/after_seq filtering + pagination across memory/db/jsonl,
the /events route forwarding, stepsForDisplay rules, and fetchSubtaskSteps
pagination. Docs updated in both AGENTS.md.
* make format
* fix(subagents): capture full multi-tool step tail, batch step persistence, cap tool-call args (#3779)
Address PR review findings on the subagent step-history feature:
1. executor.py streamed on stream_mode="values" and captured only
messages[-1] per chunk, so a multi-tool-call turn (ToolNode appends
one ToolMessage per call in a single super-step) lost all but the last
tool output in both the live task_running stream and the persisted
history. Replace with capture_new_step_messages, which walks the
newly-appended tail (and still re-checks the trailing message on
no-growth chunks so id-less in-place replacements survive).
2. worker.py persisted each step with the store's low-frequency put()
(a per-thread advisory lock per call); a deep subagent (max_turns=150)
emits hundreds of steps on the hot stream loop. Replace with
_SubagentEventBuffer, which batches via put_batch (flush on terminal
subagent.end, at FLUSH_THRESHOLD, and in the worker finally).
3. build_subagent_step capped only text; tool_calls[].args were copied
verbatim, so a large write_file/bash payload produced an unbounded
subagent.step row. Cap each call's serialized args at
SUBAGENT_STEP_MAX_CHARS, flagged args_truncated.
Tests updated/added for all three; AGENTS.md refreshed.
* fix(subagents): merge backfill into latest subtask state; reuse message_content_to_text (#3779)
Address the remaining two PR review findings:
4. subtask-card's fetchSubtaskSteps().then(updateSubtask) closed over a
stale tasks snapshot: a late-resolving backfill wrote setTasks({...stale}),
clobbering SSE steps/status and sibling subtasks that arrived during the
fetch. useUpdateSubtask now reads/writes through a tasksRef mirroring the
latest state (ref-to-latest), and the pure per-subtask transition is
extracted to core/tasks/subtask-update.ts::computeNextSubtask (unit-tested).
5. step_events._content_to_text duplicated deerflow.utils.messages.
message_content_to_text; call the shared helper instead (guarding None
content with 'or ""' so a tool-call-only turn still renders as "").
Tests added for computeNextSubtask and the None-content case; AGENTS.md docs updated.
* fix: generate title for interrupted first turn
* test(title): cover partial-exchange + dict-form messages
Harden the interrupted-run fallback path added in 19fc34fd:
- TitleMiddleware._should_generate_title now accepts a lone first-turn
user message when allow_partial_exchange=True, so the worker can still
derive a title if cancellation lands before any AI chunk is checkpointed.
- runtime/runs/worker._ensure_interrupted_title computes the next
checkpoint step defensively (treat missing/non-int step as 0) and
renames a shadowed ckpt_config local for readability.
- Add four unit tests in tests/test_title_middleware_core_logic.py:
partial-exchange allows user-only, partial-exchange still respects an
existing title, dict-form messages are recognized, and the sync
fallback path derives a title from dict-form messages — matching what
channel_values stores in the checkpoint.
Refs #3859.
* fix: persist interrupted-title via channel_versions bump
Address PR #3874 review feedback: ``_ensure_interrupted_title`` previously
called ``aput(..., new_versions={})``. LangGraph's DB-backed savers
(``PostgresSaver`` and the v4 ``SqliteSaver`` blob layout) strip inline
``channel_values`` from ``put`` and only persist blobs for channels named
in ``new_versions`` — so the fallback ``title`` channel was dropped on
read-back and ``threads_meta.display_name`` stayed ``"Untitled"`` after
refresh on those backends. The original in-memory e2e passed because
``InMemorySaver`` keeps the inline snapshot verbatim.
Fix mirrors ``_rollback_to_pre_run_checkpoint`` in the same file: bump
``channel_versions["title"]`` (via ``checkpointer.get_next_version`` when
available, else int/string fallbacks), persist the new version on the
checkpoint, and declare it in ``new_versions`` so the DB savers actually
write the blob.
Regression coverage in ``tests/test_run_worker_rollback.py``:
- ``test_ensure_interrupted_title_bumps_channel_version_and_declares_it_in_new_versions``
— exact ``aput`` invariants: ``new_versions == {"title": 1}``, the
written checkpoint's ``channel_versions["title"]`` is bumped, and the
pre-existing ``messages`` version is preserved.
- ``test_ensure_interrupted_title_bumps_existing_string_version`` —
string-shaped prior version (some savers use UUID-style versions);
bumped value must differ from the prior, no overwrite-in-place.
- ``test_ensure_interrupted_title_skips_when_title_already_set`` — title
short-circuit; no extra ``aput``.
- ``test_ensure_interrupted_title_returns_none_when_no_checkpoint`` —
no checkpoint yet; returns ``None`` without writing.
- ``test_ensure_interrupted_title_round_trip_with_real_sqlite_checkpointer``
— full round-trip against a real ``AsyncSqliteSaver`` on a disk-backed
DB, then closes and re-opens the saver to simulate a fresh
connection. The fallback title must still be present on the second
``aget_tuple``. This is the exact scenario the review flagged.
Validated locally with the full backend suite: 5195 passed, 18 skipped.
Refs #3859. Addresses review on #3874.
* test(worker, title): harden interrupted-title fallback for every saver
Defensive coverage on top of the channel_versions fix (commit 05253957),
addressing edge cases surfaced during a second-pass review of #3874.
Worker:
- Extract version bump into ``_bump_channel_version(checkpointer, current)``
with explicit fallbacks for int / float / numeric-string / UUID-shaped
string / None / bool, AND a wrap-around defense when the saver's
``get_next_version`` raises or returns an unchanged value. The
invariant is: returned version MUST differ from the prior. Without
this, a saver bug (or a custom backend) could leave
``new_versions={"title": v}`` no-op on DB savers — the very class of
bug the original review pointed out.
Title middleware:
- Coerce ``state.get("messages")`` from ``None`` to ``[]`` on both
``_should_generate_title`` and ``_build_title_prompt``. A
partially-initialized checkpoint can carry ``messages=None`` on the
channel_values channel (the worker reads raw channel_values, not
BaseMessages), and the default kwarg only protects against a missing
key. Repro: ``TypeError: 'NoneType' object is not iterable`` from
the next() generator — confirmed by reverting the fix and watching
``test_*_handles_none_messages_channel`` go red.
Tests (TDD-verified red→green for the new asserts):
- ``test_run_worker_rollback.py``:
* ``_bump_channel_version`` — 8 tests covering every version type
(int, float, numeric string, UUID-style string, None, bool) and
every saver-side fault mode (no ``get_next_version`` / raising /
stuck on identity).
* ``test_ensure_interrupted_title_*`` — 5 additional helper
boundary tests: title.enabled=false short-circuit; empty
messages list; messages=None; aput-error propagation (helper
contract: caller swallows, not the helper); idempotency on a
real InMemorySaver across two invocations.
* ``test_ensure_interrupted_title_preserves_non_title_channel_versions``
— pins that ``new_versions`` only contains ``"title"`` and that
other channels' versions are untouched (regression anchor for a
sloppier draft that bumped every channel).
* ``test_worker_finally_block_swallows_helper_exceptions`` — pins
the integration contract: even if the helper raises, the worker's
threads_meta status sync still runs and ``publish_end`` is still
awaited so the SSE stream closes cleanly.
- ``test_title_middleware_core_logic.py``:
* 4 additional tests: ``messages=None`` on both
``_should_generate_title`` and ``_build_title_prompt``; the
``role: user`` / ``role: assistant`` (OpenAI-style) dict
normalization; partial-exchange path with a dict-form message.
Verification:
- ``PYTHONPATH=. uv run pytest tests/ -x --ignore=tests/blocking_io -q``
→ 5215 passed, 18 skipped.
- ``ruff check`` + ``ruff format --check`` clean on every touched file.
- Red/green TDD verification: temporarily reverted the
``new_versions={}`` fix → 4 new tests went red as expected; restored
and the suite is green again. Same red/green dance for the
``messages=None`` coercion.
Refs #3859. Addresses second-pass review on #3874.
* fix(title): ignore dict context reminders in fallback
* fix(worker): link interrupted-title checkpoint to its parent
The title-bump checkpoint written by ``_ensure_interrupted_title`` was
landing without a ``parent_checkpoint_id`` — a real orphan in the
LangGraph history graph. Reproduction (disk-backed AsyncSqliteSaver):
[seed] checkpoint_id = 1f173dbc...
[helper] wrote title = "Why is the sky blue?"
[issue 1] new checkpoint = 1f173dbc..., parent = None
[issue 1] is new checkpoint orphaned? True
Root cause: ``_ensure_interrupted_title`` built ``write_config`` as
``{"thread_id": ..., "checkpoint_ns": ...}`` only. ``BaseCheckpointSaver``
implementations read ``configurable.checkpoint_id`` from that config as
the *parent* id when inserting (see ``langgraph/checkpoint/sqlite/aio.py``
``aput``: ``config["configurable"].get("checkpoint_id")`` becomes the
``parent_checkpoint_id`` column). With no value, the saver writes NULL —
the new checkpoint is a tree root.
Consequences:
- Any future LangGraph ``runs.resume_from`` / time-travel feature has no
backward edge to walk past the title-bump.
- History-visualization UIs built on ``alist()`` render the title-bump as
a sibling of the prior checkpoint, not its descendant.
Fix: read ``checkpoint_id`` off the tuple's own config and thread it into
``write_config["configurable"]["checkpoint_id"]`` before calling ``aput``,
the same pattern every middleware-driven write uses.
Three new regression tests against real ``AsyncSqliteSaver`` (disk-backed,
fresh connections so we exercise the on-disk read path):
- ``test_ensure_interrupted_title_links_new_checkpoint_to_its_parent`` —
asserts ``latest.parent_config["configurable"]["checkpoint_id"]`` equals
the seeded checkpoint id. TDD red-green verified: reverting the fix
flips this test red with ``AssertionError: title-bump checkpoint must
have a parent_config``.
- ``test_ensure_interrupted_title_appears_in_history_with_audit_marker``
— pins the audit contract: the title-bump entry in ``alist()`` carries
``metadata.source == "update"`` and ``metadata.writes`` contains
``runtime_interrupt_title``. This is a deliberate design choice — we
do NOT hide the entry from history (audit trail belongs in the saver),
but its source and writes marker MUST be unambiguous so UIs/tools can
identify it.
- ``test_ensure_interrupted_title_survives_immediate_next_turn`` —
cancel → immediate user follow-up scenario. Simulates the agent's next
turn appending a (user, ai) pair without touching the title channel,
then opens a fresh saver and verifies the title is still present after
the next-turn checkpoint write. Pins the channel-version-blob
invariant established by commit 05253957 — without the
``new_versions={"title": v}`` declaration there, the title blob would
vanish from the DB and this test would read back ``None``.
Verification:
- ``PYTHONPATH=. uv run pytest tests/ -x --ignore=tests/blocking_io -q``
→ 5222 passed, 15 skipped.
- ``ruff check`` + ``ruff format --check`` clean on every touched file.
- Reproduction script confirms ``parent_checkpoint_id`` is now non-null
and the next-turn read-back preserves the fallback title.
Refs #3859.
* Revert "fix(worker): link interrupted-title checkpoint to its parent"
This reverts commit c763ed9334781db1acdce0f5f33d663d8d5f80ff.
* test: trim over-engineered test coverage
Reduce review surface area on PR #3874 by dropping defensive tests that
don't pin a real invariant. After self-review:
- ``_bump_channel_version``: 8 tests → 2 (happy path + saver-error
fallback). Dropped float / bool / numeric-string / UUID-string /
missing-get-next-version / stuck-get-next-version branches — those
are speculative scaffolding for savers we don't ship.
- ``_ensure_interrupted_title``: dropped ``returns_none_when_title_disabled``,
``returns_none_with_no_user_message``, ``returns_none_when_no_checkpoint``
— boundary guards already exercised by the e2e test and the
``handles_none_messages_channel`` regression anchor.
Net: -107 lines of test code. Remaining coverage still pins every
red-green-verified invariant (channel_versions bump, string-version
bump, idempotency, sqlite round-trip, non-title channel preservation,
aput-error contract, worker finally swallowing, partial-exchange).
Verification: 5209 passed, 15 skipped.
* fix: harden interrupted title finalization
* fix: serialize interrupted title finalization
* fix: preserve interrupt semantics during title finalization
* fix: preserve delayed interrupted title recovery
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* feat(community): add Browserless web_capture screenshot tool
Add a web_capture tool that renders a page via Browserless /screenshot and
presents it through the artifact system, alongside the existing Browserless
web_fetch provider.
Hardening:
- SSRF guard: reject URLs resolving to private/loopback/link-local (incl. the
169.254.169.254 cloud-metadata endpoint)/reserved/multicast/unspecified
addresses; opt out via allow_private_addresses for internal targets.
- Surface a warning when Browserless renders a target page that itself
responded with a non-2xx/3xx status (X-Response-Code), so an error/anti-bot
page is not mistaken for valid visual evidence.
- Dedupe colliding output filenames instead of silently overwriting prior
captures.
Docs: comment out token: $BROWSERLESS_TOKEN in tool examples (an unset $VAR
fails AppConfig startup) and document allow_private_addresses.
* fix(community): format web_capture guard + document local Browserless startup
Address PR #3881 review: fix the lint-backend failure (ruff format on
browserless/tools.py) and add local Browserless startup instructions to
CONFIGURATION.md so reviewers can run the service to try web_fetch/web_capture.
* feat(scripts): add redacted community support bundle generator
Add `make support-bundle` (scripts/support_bundle.py) to help users file
high-signal, privacy-safe GitHub issues for local setup/config/runtime
problems.
The command produces:
- `*-issue-summary.md` to paste into the issue body
- `*-issue-draft.md` scaffold for AI-assisted filing (REQUIRED placeholders,
never invents repro/expected/summary facts)
- an optional evidence zip under `.deer-flow/support-bundles/` containing a
stable `triage.json` plus redacted environment/config/extensions/git/doctor
evidence
Privacy: secrets are redacted across config values, URL userinfo, query
strings, CLI flags, custom headers, bearer/sk- tokens, and home paths. The
bundle never includes `.env`, raw conversation messages, or user file
contents; optional `--thread-id` adds file manifests only. `thread_id` input
is validated against path traversal.
Wire it into the Makefile, AGENTS.md, README/README_zh, CONTRIBUTING, and the
bug-report issue template. Covered by backend/tests/test_support_bundle.py.
* fix(scripts): redact MCP env values by default in support bundle
Address PR #3886 review (willem-bd, P2): the key-name allowlist let literal
secrets under non-standard env keys (e.g. SUPABASE_SERVICE_ROLE_KEY,
R2_ACCESS_KEY, hardcoded AIza… keys) leak verbatim into the bundle that users
are told is safe to share publicly.
Mask all MCP `env` values by default, keeping only `$VAR`/`${VAR}` references
visible, and broaden SECRET_KEY_RE (access_key, pwd, private_key). Add tests
for non-keyword env secrets, broadened key names, and end-to-end zip redaction.
Adds ``deerflow.community.e2b_sandbox.E2BSandboxProvider`` with parity
to AioSandboxProvider: metadata-keyed per-thread persistence, server-
side idle timeout, warm-pool reclaim with liveness checks, /mnt/user-data
bootstrap symlinks, dead-sandbox auto-rebuild, and release-time mirror
of agent outputs back to the host artifact directory.
Signed-off-by: joey <zchengjoey@gmail.com>
* feat(subagents): add delegations ledger field + reducer to ThreadState
* feat(subagents): pure helpers to derive + format the delegation ledger
* feat(subagents): DelegationLedgerMiddleware records + injects the ledger
* feat(subagents): register DelegationLedgerMiddleware for lead when subagents enabled + docs
* add runtime log
* chore(subagents): make delegation-ledger injection log production-ready
* test(subagents): make delegation-ledger registration tests config-free; refresh ultra replay golden for delegations channel
* refactor(subagents): derive TERMINAL_STATUSES from SUBAGENT_STATUS_VALUES + pin it
Make thread_state's TERMINAL_STATUSES a frozenset over the status contract's
SUBAGENT_STATUS_VALUES instead of a hardcoded literal, so the terminal-status
set can never drift from the contract. Add a pinning test asserting the
derivation and that the non-terminal "in_progress" stays excluded.
Addresses PR #3877 review.
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* fix(sandbox): stop blocking bash commands from hanging the turn
Starting a server through the host bash tool (e.g. `python -m http.server`)
could hang the whole turn for the full 600s timeout. `LocalSandbox.execute_command`
used `subprocess.run(capture_output=True)`, whose captured pipes are inherited
by any process the command spawns — so a backgrounded long-lived process
(`server &`) keeps the read end open and blocks `communicate()` until the
timeout fires, even though the foreground command already returned. Commands
that read stdin blocked the same way, and on timeout only the direct child was
killed, leaving orphaned process groups.
Rework the POSIX path to capture stdout/stderr via temp files instead of pipes,
take stdin from /dev/null, and run the command in its own session/process group:
- Backgrounded long-lived processes (servers) now return immediately while the
process keeps running.
- A command reading stdin gets immediate EOF instead of blocking.
- A genuinely blocking foreground command is bounded by a configurable
wall-clock timeout; on timeout the whole process group is killed and the agent
gets an explanatory notice telling it to background long-lived processes.
The timeout is configurable via `sandbox.bash_command_timeout` (default 600).
The Windows path is unchanged. Adds focused regression tests and updates docs.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(sandbox): instruct the agent to background long-lived processes
The code fix bounds a foreground server with a timeout, but the turn still
waits the full timeout before the run continues. Add the prompt-side half:
the bash tool description now tells the model to ALWAYS start long-lived
processes (e.g. web servers) in the background with output redirected, so the
tool returns immediately. The timeout notice points at the same readable
workspace log path. Pins the guidance with a test.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(sandbox): make fallback-kill exception explicit and observable
Address automated review: the inner `except OSError: pass` in
_terminate_process_group silently swallowed the case where the direct-child
fallback kill found the process already gone. Make the intent explicit with a
comment and a debug log instead of a bare pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(sandbox): address bash timeout review feedback
* fix(sandbox): document fd cleanup races
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Split `_build_runtime_middlewares`'s flat list into three named declarative
sublists (outer_wrappers / thread_hooks / tail) and drop the
`middlewares.insert(2, UploadsMiddleware())` magic-index pattern. The
declarative structure makes the layering self-documenting and immune to
position drift when the head of the list changes.
Move UploadsMiddleware to run after ThreadDataMiddleware in the chain.
Under the previous order (a magic-index artifact introduced when #3662
prepended InputSanitizationMiddleware), UploadsMiddleware scanned the
uploads directory before ThreadDataMiddleware created it under
lazy_init=False, so historical files could be missed on the first run of
a thread. Narrow path — the upload endpoint normally pre-creates the
directory — but the order is the correct semantic and is now locked.
Documentation:
- backend/AGENTS.md middleware chain renumbered: ThreadDataMiddleware is
now #3, UploadsMiddleware #4 (was reversed).
Tests (backend/tests/test_tool_error_handling_middleware.py):
- test_build_lead_runtime_middlewares_orders_thread_data_before_uploads
— focused td_idx < um_idx assertion.
- test_build_lead_runtime_middlewares_chain_order_matches_agents_md
— full-chain order pin using real classes, so a swap between any pair
is caught (the existing FakeMiddleware-stubbed tests cannot detect
this).
- test_lead_runtime_chain_finds_historical_uploads_under_lazy_init_false
— integration anchor: under lazy_init=False, ThreadDataMiddleware
creates the dirs, then UploadsMiddleware surfaces a pre-existing
historical file in the injected <uploaded_files> context.
* fix(feishu): stop creating thread topics and throttle card updates
- Remove reply_in_thread(True) so replies appear as normal messages (#1332)
- Use chat_type=p2p for shared LangGraph thread in P2P chats
- Add two-level stream throttle (1.0s interval / 60 char buffer) with OR logic
- Add cursor indicator for streaming
- Filter values from overwriting delta text during streaming
Closes#3801
* fix(feishu): stop creating thread topics, throttle card updates, preserve clarification
- Remove reply_in_thread(True) so replies appear as normal messages (#1332)
- Use chat_type=p2p for shared LangGraph thread in P2P chats, with stored
mapping fallback for backward compatibility with pre-upgrade threads
- Add two-level stream throttle (1.0s interval / 60 char buffer) with OR
logic and cursor indicator for non-final outbounds
- Re-publish clarification text from values snapshots mid-stream
- Update streaming tests to the new contract (cursor glyph, clarification)
Closes#3801
get_or_new_skill_storage() and reset_skill_storage() touch the
process-global skill storage singleton without a lock - the same
unsynchronized check-then-create that #3730 just fixed in
sandbox_provider.py, the module this file documents itself as mirroring.
Two callers racing a cold start can both see _default_skill_storage is
None and each build a SkillStorage, so the second overwrites the first;
a reset_skill_storage() racing a get can also null the global between
the None-check and the return.
Guard the build/return and the reset with a module-level threading.Lock
and a double check, mirroring get_memory_storage(). Construction stays
inside the lock (rather than sandbox_provider's build-outside-then-
discard-the-loser) because SkillStorage has no teardown hook, so a
losing racer's orphan could not be cleaned up.
Add backend/tests/test_skill_storage_lifecycle.py with concurrency
regression tests (8-thread cold-start race asserting a single instance;
reset racing gets asserting no None is returned).
Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com>
* feat(tui): add Hermes-like terminal workbench backed by DeerFlowClient
Implements the `deerflow` TUI from RFC #3540: a terminal-native, embedded
workbench over the existing harness (no Gateway/frontend/nginx/Docker), built
Python-native with Textual and learning UX patterns from tao-pi.
Architecture — every layer except the Textual app is pure and unit-tested:
- view_state.py: ViewState + reduce(state, action), the testable heart
- runtime.py: StreamEvent -> reducer actions (pure translate + threaded driver)
- message_format / command_registry / input_history / render / theme: pure
- app.py: Textual App; runs the sync DeerFlowClient.stream() on a worker thread
and marshals actions back to the UI thread. Slash command palette, model and
thread modal pickers, ↑/↓ history, Ctrl+C interrupt, TTY-aware fallback.
- cli.py: pure launch-mode planning + headless --print/--json + `deerflow`
console script (textual is an optional [tui] extra; degrades to headless help)
Web UI visibility (the RFC's key decision): persistence.py writes a threads_meta
row under the local default user into the same DB the Gateway reads, so terminal
sessions appear in the Web UI sidebar without running the Gateway. Best-effort,
no-op on the memory backend; all DB work on one long-lived background loop.
Tests: 95 TUI tests — pure layers via pytest, app/palette/overlays via Textual's
pilot harness with a fake session, and a threads_meta read/write round-trip.
ruff clean; respects the harness->app import boundary. Docs: backend/docs/TUI.md
plus CLAUDE.md/README updates and preview screenshots.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(tui): de-duplicate streamed assistant text and tool cards; keep Tab in composer
Self-test surfaced three issues, all root-caused to consuming non-strict
streaming from DeerFlowClient (proven by the client's own
test_dedup_requires_messages_before_values_invariant, which shows the client can
re-emit a message id's full content twice):
- Assistant text was doubled (e.g. "answer answer") because the reducer blindly
concatenated same-id deltas. Now merges by content: a re-send or cumulative
snapshot replaces; only genuine increments append.
- Tool activity showed duplicate and empty "gear" cards from partial/re-emitted
tool-call chunks. ToolStarted now de-dupes by tool_call_id, drops id-less
noise chunks, and fills the name on a later chunk; a tool result with no prior
card still surfaces as a completed card.
- Tab moved focus off the composer to the scroll region (felt like broken cursor
logic). Tab is now consumed by the composer (completes a command when the
palette is open, no-op otherwise).
Adds reducer tests for each case plus a Tab-focus test; 102 TUI tests pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(tui): make Esc interrupt an active run (matches the status hint)
The status line advertised "esc interrupt" but Esc was only wired to close the
slash palette, so it did nothing during a run. Esc now: closes the palette when
open, interrupts the active run when streaming, and is a no-op when idle. The
interrupt logic is shared with Ctrl+C via _interrupt_run(). Adds a regression
test.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(tui): stop prior answers duplicating on threads with history
On a thread with history, DeerFlowClient re-emits every prior message on each
new turn (its streamed_ids dedup is per-stream-call), and a re-emitted older
message can arrive after a newer message has already started. The reducer only
matched the *most recent* assistant row by id and otherwise appended, so each
re-emitted older answer was duplicated verbatim at the end of the transcript.
Match an assistant row by id anywhere in the transcript and merge in place.
Tool cards already de-dupe by call id globally, so they were unaffected.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(tui): correct CJK cursor drift in the composer
Confirmed a Textual Input bug (latest 8.2.7): Input._cursor_offset adds an
unconditional +1 at the end of the value, overshooting by one cell after
double-width (CJK) characters. That misplaces the hardware/IME cursor — the
drift seen when typing Chinese in iTerm2 (the on-screen block cursor, drawn
separately in render_line, is fine; English doesn't use an IME so it looks
correct). Reproduced with a bare Input, so it's upstream, not our layout.
Add ComposerInput(Input) overriding _cursor_offset to the true cell position and
use it for the composer. Numeric tests pin the CJK end/mid and ASCII cases.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(tui): render finalized assistant messages as Markdown
The transcript showed raw Markdown (literal **bold**, ## headings, - lists,
links). Finalized assistant messages now render as Rich Markdown — headings,
bold/italic, lists, inline code + code blocks, blockquotes, horizontal rules and
links — with the ● speaker marker aligned to the top of the body.
The actively-streaming message stays plain text so partial Markdown doesn't
reflow/jump, then snaps to its rendered form when the run ends. Transcript
re-renders are coalesced on a ~60ms timer (dirty flag) so per-token Markdown
re-parsing stays smooth on long threads. Tests cover both the rendered and the
streaming-plain paths.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* style(tui): apply ruff format
CI lint runs `ruff format --check` via uvx (latest ruff); apply the formatter so
the lint-backend job passes. No behavior change.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* chore(tui): address code-quality review comments
From github-code-quality[bot] on #3760:
- runtime.py: give the `_ClientLike` Protocol method a docstring body instead of
a bare `...` (flagged as a no-effect statement), matching the harness
convention for Protocol stubs (e.g. SafetyTerminationDetector).
- test_tui_cli_main.py: drop the unnecessary `lambda: _FakeSession()` wrappers in
monkeypatch.setattr; pass `_FakeSession` directly (same behavior).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(tui): keep history Markdown-rendered when a follow-up run starts
Previously the transcript rendered "the last assistant row" as plain text while
streaming. But when a follow-up turn starts, the last assistant row is the
*previous, finalized* answer until the new message begins — and the client
re-emits prior messages early in the turn — so sending a follow-up reverted the
previous answer from rendered Markdown back to raw text.
Track the actively-streaming message id in ViewState instead: it's reset on
RunStarted, set only when an AssistantDelta actually adds new content (history
re-emits are no-ops and don't mark it), and cleared on RunEnded. The renderer
keeps only that one message plain; all history stays Markdown.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs(readme): add Terminal Workbench (TUI) section to root README
Mention the new `deerflow` TUI alongside the Embedded Python Client in the root
README.md and README_zh.md (install, launch/headless commands, feature summary,
Web UI visibility), with a ToC entry and a preview screenshot. Links to
backend/docs/TUI.md for the full guide.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(tui): address review feedback (willem-bd)
Ten findings from the TUI code review:
1. /resume was dead-ended — registered + in /help + tested as a builtin, but no
dispatch branch. Wired it to thread resolution / the switcher.
2. --resume <title> was forwarded raw into the checkpointer (blank thread).
Added Session.resolve_ref() to resolve id-or-title via list_threads; used by
--resume and /resume.
3. str(get("id","")) returned "None" for an explicit id:None (truthy), defeating
the empty-id guard so unrelated null-id tool calls collapsed into one card.
Coerce via a None-safe helper.
4. Headless --print/--json no longer spin up the persistence loop/engine/pool
(open_session(persistence=False)).
5. _LoopThread + engine are now closed: Session.close() (dispose engine + stop
loop) called from a try/finally around app.run().
6. --cli --continue (and piped --cli) now run headless instead of erroring.
7. Cancelled runs no longer persist a truncated title (guard on _cancelled).
8. Palette highlight resets to the top when the filter set changes.
9. Dropped the never-populated tools count from the header.
10. Documented the `not row.error` merge guard.
Adds regression tests for each; 126 TUI tests pass, ruff check + format clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* fix(sandbox): synchronize sandbox provider singleton lifecycle
get_sandbox_provider() used an unsynchronized check-then-create, so two OS
threads (e.g. the main event loop and the Feishu channel thread, which runs
its own loop) could double-initialize the provider. With AioSandboxProvider
the overwritten instance leaks its idle-checker thread, since the only code
that joins it (shutdown()) is reachable only through the reference that was
overwritten.
reset_sandbox_provider(), shutdown_sandbox_provider() and set_sandbox_provider()
also touched the global without a lock, so a reset/shutdown racing an in-flight
create could clear it mid-construction or tear down an instance another thread
was about to return.
Guard all four lifecycle sites with a single module-level threading.Lock and
use double-checked locking in the getter, mirroring get_memory_storage().
* test(sandbox): add concurrent regression tests for provider singleton
- 8 threads racing on cold start, synchronized with a threading.Barrier so
the check-then-create race fires deterministically; asserts exactly one
provider instance is created.
- reset racing concurrent gets: asserts every returned value is a fully
constructed provider (never None / half-built).
* fix(sandbox): lock get read path, run provider callbacks outside the lock
Addresses the three review findings on #3730:
1. get_sandbox_provider()'s read+return ran outside _provider_lock, so a
concurrent reset/shutdown/set could null or tear the global between the
check and the return, handing callers None / a torn instance. The hot read
and the install reconciliation now both happen under the lock.
2. The non-reentrant _provider_lock was held across plugin-supplied callbacks
(resolve_class import + provider __init__ in get; provider.reset()/shutdown()
in reset/shutdown). A custom provider that re-entered these lifecycle
functions would self-deadlock, and a slow teardown blocked every concurrent
get(). resolve_class + construction now run outside the lock; reset/shutdown
detach the reference under the lock and invoke the callback outside it.
Tradeoff: racing cold-start callers may each construct a candidate. Exactly
one is installed and returned to everyone; the losers (e.g. an AioSandbox
instance that already started an idle-checker thread) are shut down so they
do not leak the orphan thread #3721 is about. set_sandbox_provider() documents
that it replaces but does not shut down the prior instance.
3. test_reset_racing_get reset the singleton to None *before* the barrier, so
the racing reset was a no-op and never exercised reset-of-a-live-provider.
It now populates the singleton up front so the reset tears down a live
instance while getters read it.
Tests: rename the cold-start test to assert "one installed singleton, observed
by all" (construction is no longer single under the new design); add
shutdown-vs-get, set-vs-get, and a losing-racer-shuts-down-its-orphan case.
All five pass; the existing sandbox/middleware/mounts/uploads suites are green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Add endswith('__user') guard in _is_user_injection_target to prevent
unbounded suffix growth (id__user__user__user...) when a prior ID-swap
peer is mistakenly treated as a new injection target
- Add peer rescue in _preserve_dynamic_context_reminders to keep
ID-swap __user and __memory messages out of summary compression,
preventing orphaned SystemMessage reminders and lost user context
- Add 6 regression tests covering both failure modes
* perf(runtime): index MemoryRunEventStore events by run_id to avoid O(n) scans
list_messages was already served from a thread-wide messages projection
(#3531), but list_events and list_messages_by_run still scanned the whole
thread's event log (every run, every category) to return one run's events --
O(N_thread) on every run-scoped /messages page-load and /events request.
Add _events_by_run / _messages_by_run projections (same dict objects, kept
in lockstep in _put_one / delete_by_run / delete_by_thread), so both reads
are O(M_run), with bisect cursor pagination for messages. Semantics are
unchanged, pinned by a brute-force parity test over interleaved traces and
both cursors.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* style: ruff format test_run_event_store_by_run_index.py
Clears the lint-backend (ruff format --check) failure on the PR; the original
commit ran `ruff check` but not `ruff format`. No behavior change (test fixture
formatting only).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: ly-wang19 <ly-wang19@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(persistence): wire alembic migrations + bootstrap schema on startup
Closes#3682. Pre-#3658 DBs lack the `runs.token_usage_by_model` column
because alembic was never wired up — startup only ran `create_all`,
which never ALTERs existing tables.
Adds a hybrid bootstrap in FastAPI lifespan (replaces bare `create_all`):
- empty DB → create_all + stamp head
- legacy DB → stamp 0001_baseline + upgrade head
- versioned DB → upgrade head
Concurrency: Postgres `pg_advisory_lock` (cross-process); SQLite
per-engine `asyncio.Lock` + 30s `PRAGMA busy_timeout` on both prod and
alembic engines. Column revisions use `safe_add_column` /
`safe_drop_column` idempotent helpers as fallback.
Other bits:
- 0001 baseline (chain root) + 0002 add `runs.token_usage_by_model`
- `include_object` filter so alembic ignores LangGraph checkpointer tables
- `make migrate-rev MSG="..."` for authoring new revisions
(no migrate/stamp targets — startup is the only execution path)
- Tests: three-branch decision, concurrency, #3682 regression, env
filter, blocking-IO gate anchor
- CLAUDE.md: new "Schema migrations" section
* fix(style): fix lint error
* perf(persistence): address review feedback on alembic bootstrap
Behavioural fixes
- _SQLITE_LOCKS now keyed via WeakKeyDictionary so id-reuse after GC
cannot return a stale, loop-bound lock and the cache cannot leak one
entry per disposed engine.
- safe_add_column compares nullable / server_default against the desired
column when the name already exists and emits a warning on drift,
surfacing manual-ALTER workarounds instead of silently no-op'ing.
- _postgres_lock issues SET LOCAL idle_in_transaction_session_timeout=0
before pg_advisory_lock, so managed Postgres cannot kill the idle
lock-holding session mid-upgrade and silently release the advisory
lock.
- legacy branch now backfills missing baseline tables via a restricted
create_all (Base.metadata.create_all scoped to _BASELINE_TABLE_NAMES).
Restores pre-#1930 upgraders whose channel_* tables were never
provisioned, without pre-empting future create_table revisions for
newly-added models.
Schema parity
- runs.token_usage_by_model gains server_default=text("'{}'") in both
the ORM model and the 0001_baseline create_table, matching what 0002
adds via ALTER. create_all and alembic-upgrade paths now produce
identical column definitions.
- New parity test compares Base.metadata.create_all output against a
pure alembic upgrade base->head, asserting column-set, nullable, and
server_default agree across all tables (normalized through the same
helper safe_add_column's drift check uses).
Guards
- test_baseline_table_names_constant_matches_0001 pins
_BASELINE_TABLE_NAMES to 0001_baseline.upgrade()'s actual output --
the constant cannot drift silently when someone edits 0001.
- test_legacy_backfill_skips_non_baseline_tables verifies the restricted
backfill does not create a phantom table on Base.metadata, modelling
a future revision that would otherwise collide on op.create_table.
Doc residuals
- Three-branch decision table is now consistent across bootstrap.py
top docstring, engine.py comment, test module docstring, and
CLAUDE.md.
- Stale test anchor in blocking_io/test_persistence_engine_sqlite.py
docstring now points at the real file.
* fix(style): fix lint error
* fix(persistence): close drift detection holes
- _check_column_drift compares column type via a family equivalence
allowlist ({JSON, JSONB}). Catches the wrong-type workaround
`TEXT NOT NULL DEFAULT '{}'` that previously slipped through silently,
while keeping Postgres JSON/JSONB dialect reflection quiet. Reflected
and desired type are also echoed in every drift warning's payload for
operator triage.
- Extract _escape_url_for_alembic so bootstrap._alembic_safe_url and
scripts/_autogen_revision share the ConfigParser % escape rule
instead of duplicating it.
- backend/README.md: add `make migrate-rev MSG=...` to Commands and a
Schema Migrations section per the repo's README/CLAUDE.md sync policy.
- test_base_to_dict.py: scope the test ORM class to an isolated MetaData
so the create_all-vs-alembic parity test (added in the previous
commit) is not polluted by the phantom table on the full pytest
session.
* perf(runtime): index MemoryRunStore by thread_id to avoid O(n) scans
MemoryRunStore is the default run backend (database.backend=memory) and backs
RunManager.list_by_thread, which calls it on every thread-runs query to hydrate
persisted runs. list_by_thread scanned every run in the store (O(total runs))
to filter by thread_id, so listing one thread's runs got linearly slower as
unrelated runs accumulated across all threads.
Add a thread_id -> insertion-ordered run_id set secondary index, maintained in
lockstep with _runs in put()/delete(), and use it in list_by_thread for an
O(runs-in-thread) lookup. This mirrors the index RunManager already keeps over
its own in-memory records (#3499); the store extraction — whose docstring notes
it is "Equivalent to the original RunManager._runs dict behavior" — did not
carry the index across.
Behavior is unchanged: same user_id filtering, newest-first ordering, and limit.
The store runs each method without awaits on the event loop, so the index and
_runs stay consistent without a lock.
Extends tests/test_persistence_scaffold.py::TestMemoryRunStore with coverage for
unknown-thread, newest-first ordering, limit, and index cleanup on delete
(including empty-bucket removal).
* perf(runtime): route aggregate_tokens_by_thread through the thread index too
list_by_thread already uses the _runs_by_thread index this PR adds, but
aggregate_tokens_by_thread (the /token-usage endpoint) still scanned every run
in the process to pick out one thread's runs. Route it through the same index
for an O(runs-in-thread) lookup, completing the thread-scoped read coverage.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: ly-wang19 <ly-wang19@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
get_artifact ran its filesystem work directly on the event loop: virtual-path
resolution (os.path.abspath via .resolve()), exists/is_file probes, MIME sniffing
(mimetypes lazily stats the system MIME DB on first use), full-file
read_text/read_bytes, is_text_file_by_content (open+read), and .skill ZIP
open+extract. So serving any artifact blocked the loop for the whole read;
`make detect-blocking-io` flagged it. Same class as #3457 / #3529.
Offload each branch's IO via asyncio.to_thread: one sync helper per branch
(_load_skill_archive_member, _read_artifact_payload) folds stat + MIME + read /
extract into a single worker hop and returns a small (kind, mime, payload) plan
the handler turns into the response on the loop. FileResponse (download / active
content) keeps streaming the file itself. Behavior, branching, error codes, and
security boundaries are unchanged.
Add tests/blocking_io/test_artifacts_router.py anchor (text / binary / .skill
member), verified red->green under the strict Blockbuster gate. The gate also
caught a blocking call the static scan missed: resolve_thread_virtual_path's
.resolve() (os.path.abspath), now offloaded too.
Co-authored-by: ly-wang19 <ly-wang19@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(channels): let UI runtime channel config win over config.yaml
Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com>
* test(channels): update assertion to expect runtime/UI value to win over yaml
The test was written when yaml took precedence over runtime config.
This PR inverts that precedence so UI-entered credentials win; the
assertion now correctly reflects that runtime value (xapp-ui) beats
the yaml value (xapp) on a shared key.
---------
Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com>
Part of #3742. RunJournal._message_text and the gateway thread-messages
helper (thread_runs._message_text) reimplemented the same 'extract display
text from a message' logic — str / list of string|{text}|nested{content}
blocks joined without a separator / mapping with text|content key. They
differed only in two ways: journal reads a BaseMessage attribute while
thread_runs reads dict-shaped run_events rows, and journal falls back to
message.text.
Add deerflow.utils.messages.message_to_text(message, *,
text_attribute_fallback=False) that handles both message shapes (attribute
or mapping content access) and gates the .text fallback behind a flag, and
have both call sites delegate. journal passes text_attribute_fallback=True;
thread_runs uses the default. Behavior is unchanged at both sites.
Verified behavior-preserving with an equivalence harness running both
original implementations vs the shared helper over 98 inputs (BaseMessage
and dict messages; str/list/mapping/None/numeric content; mixed blocks;
.text attribute present/absent/non-str) -> 0 mismatches. Added
tests/test_utils_messages.py; the journal last_ai_message extraction tests
still pass.
DynamicContextMiddleware (PR #3630, p0) uses the ID-swap technique to
inject a SystemMessage(reminder) into the middle of the conversation.
create_agent then prepends the static system_prompt as another
SystemMessage at request time, so strict OpenAI-compatible backends
(vLLM, SGLang, Qwen) and Anthropic reject the request with
'System message must be at the beginning'.
New SystemMessageCoalescingMiddleware runs in wrap_model_call — after
create_agent prepends system_prompt — and merges every SystemMessage
into a single leading one before the request reaches the provider.
Non-system messages keep their original order; the merged SystemMessage
preserves the id of the first system message. Only the request payload
is touched; checkpoint state is unchanged, so every consumer that scans
history (memory builder, journal, summarization, dynamic-context
detection) keeps working.
Mirrors the per-request coalescing already done for Claude in
claude_provider._coalesce_system_messages (PR #3702) but at a
provider-agnostic layer so every backend benefits from a single fix.
Closes#3707
* fix(middleware): fix positional fallback consuming unrelated todo when same-content list is exhausted
When next_todos contains the same content string twice (e.g. two "A"
entries), the first iteration pops the matching previous todo from
previous_by_content["A"], leaving an empty list. The second iteration
finds that empty list, treats it as falsy, and sets previous_match=None.
The positional fallback then fires unconditionally — consuming
previous_todos[index] even if it holds a completely different entry
(e.g. "B"). That marks "B" as matched, so the final loop never emits
a todo_remove action for it.
Fix: guard the positional fallback with `content not in previous_by_content`
so it only fires for genuinely new content (the key was never in previous),
not for same-content entries whose list has simply been exhausted.
Add a regression test to _build_todo_actions covering this exact case.
* style: ruff-format the token usage middleware test
Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com>
---------
Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
