mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-11-08 20:10:34 +00:00
345 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Hyeonsang Cho
|
582a632868
|
fix(agents): key read_file loop detection on its exact line window (#5486)
Layer 1 quantized read_file's line range into 200-line buckets, which erased the offset inside a bucket: every read shorter than a bucket collapsed onto its neighbours. Five sequential 40-line reads hashed identically and tripped the hard stop, ending the run with a forced final answer and stop_reason=loop_capped — on exactly the ranged reads that read_file's own truncation notice tells the model to make. Bucketing cannot separate progress from repetition in general: an equality key can only approximate range overlap, and the approximation was erasing the offset that distinguishes the two. Key on the exact window instead, with an omitted end_line kept open-ended so a bare read and an explicit start_line=1 still share one key. Repeating a single range is still caught at the same threshold, and a read loop that varies its bounds remains covered by the per-tool frequency layer. Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
alanhuangyoo
|
86406cf197
|
fix(agents): make create_deerflow_agent's subagent limit, summarization and token_budget features take effect (#5488)
* fix(agents): keep the delegation ledger and summary in create_deerflow_agent graphs The SDK factory chain had no DurableContextMiddleware. SubagentLimitMiddleware counts a run's delegations from the ledger that middleware writes, so the per-run subagent total never tripped, and DeerFlowSummarizationMiddleware keeps compacted history in summary_text, which only that middleware puts back into model requests, so a summarized factory graph lost its history. Add it after ToolErrorHandlingMiddleware, ahead of summarization, where make_lead_agent has it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(agents): make RuntimeFeatures(token_budget=True) enforce the budget The factory built TokenBudgetMiddleware from TokenBudgetConfig(), whose enabled flag defaults to False, and every hook returns early on it. A graph created with token_budget=True had no warning, no hard stop and no token_capped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(agents): coalesce system messages in create_deerflow_agent graphs DurableContextMiddleware injects its authority contract as a second SystemMessage. The lead and subagent chains pair it with SystemMessageCoalescingMiddleware because strict backends reject that; the factory now does the same. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: trim inherited harness guidance below chain limits --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Zeren Wang
|
a58ab484a6
|
feat(projects): Projects MVP Phase 2 — instructions, document shelf, promotion, trash (#5443)
* feat(projects): Projects MVP Phase 2 — instructions, document shelf, promotion, trash Implements docs/superpowers/specs/2026-09-12-projects-mvp-phase2-design.md (issue #5160, tracker #5129) in the slice order of the spec's §16. Slices: - A: ProjectsConfig + write-time 422 UTF-8 byte cap; PROJECT_CONTEXT_KEY admission pinning (both server-owned sets + worker hoist); latest-only request-scoped <project> block via DynamicContextMiddleware wrap_model_call/awrap_model_call (idempotent reassembly, reserved ID prefix + marker + provenance, never persisted); journal audit fingerprints; Instructions tab. - B: ProjectDocumentRow + migration 0023; ProjectDocumentRepository with locked check-and-set; hash-qualified immutable shelf storage with Paths helpers; upload/list/content/delete-to-trash routes; project delete trashes the shelf in-transaction; request-scoped bounded <documents> index with honest count/shown + actionable overflow note; list_project_documents/read_project_document tools registered only on pinned runs; PAT allowlist + drift guards; blocking-IO anchors. - C: shared thread-upload ingestion service (uploads router refactored to parity); POST from-thread with provenance; attach-to-thread with lock-staged copy (archived source allowed); read-only thread-files view with per-group truncation reporting. - D: restore (restored/merged/not_found/no_target/content_missing; no file moves), purge (continuous row lock across unlink/delete/commit, retryable on FS errors), retention sweep (lazy + startup, 24h orphan guard, row-side reconciliation never deletes). - E: Documents tab (shelf + conversation-files browser, provenance, archived banner, content-missing rows), /workspace/trash route, sidebar entry, composer attach handoff, i18n (en-US/zh-CN), e2e mocks + specs. Review hardening folded in (10 rounds, all with tests): - force active shelf content (HTML/XML family) to download; nosniff on artifact + content responses; unified unsandboxed-iframe PDF preview (fixes the pre-existing Chromium sandbox blank in the artifact viewer) - scope document trash to the URL project under the document lock - atomic no-overwrite filename reservation for ALL ingestion (seeded claims + os.link commit with suffix retry; same-name re-upload now unique-names instead of replacing); hidden staging only, no visible placeholders; lease cleanup on setup failure - serialize conversion under the document lock with post-lock active revalidation; drain locked filesystem work on cancellation; preserve bytes when an insert's commit state is uncertain (including trashed rows) - original-integrity checks before serving text or cached conversions; content_missing surfaced in list responses (UI reads the flag, no 409-probe); downloads always serve original bytes - bounded streaming document reads with cached char counts; shelf limits declared in middleware release identity - thread-root confinement for from-thread sources; config fallback rejects fractional/infinite values; composer counts staged attachments; pending attachments persist until submission or removal; in-flight instruction/rename edits survive save refetches; shelf and trash pagination; conversation-file and thread-files pages stay subscribed to refetches Docs: README/README_zh, backend API.md/ARCHITECTURE.md, AGENTS.md contracts, config.example.yaml projects block. Review follow-ups (head b4807477 → this revision): - The trash retention sweep is split so repeated lazy triggers stay bounded: the indexed expiry purge still runs on every trigger (GET /api/trash/documents, POST /api/trash/purge) while the O(all rows + all files) reconciliation is throttled to one run per user per 15 minutes (process-local, per-user window). The startup sweep now runs as a background task instead of blocking gateway readiness, and shutdown awaits it (bounded). - The export scrub (stripInternalMarkers) is fence- and indentation-aware like the render path, so a pasted, fenced <project>/<documents> snippet survives markdown export while real injected blocks (never fenced) are still removed. Fence regexes moved to a dependency-free leaf module to avoid the messages↔streamdown import cycle. - The artifact viewer's PDF iframe no longer carries an added title attribute (the upstream e2e contract locates it via :not([title])), and the upstream artifact-preview spec now pins the new contract: PDFs render unsandboxed, images keep sandbox="". * fix(projects): round-2 review — cancel an overrun trash sweep, restore the PDF frame title - Shutdown cancelled only the shield around the background startup sweep, so an all-users reconciliation that outlived the 5s budget kept walking rows and files while the document repo and DB engine were disposed underneath it. The wait now lives in `_shutdown_startup_trash_sweep`, which cancels the task and drains it before worker exit: the shield keeps the wait bounded, the cancel makes it final (CancelledError lands at the sweep's next await, and `_run_startup_trash_sweep` only catches `Exception`, so nothing swallows it). - The browser-preview iframe lost `title={getFileName(filepath)}` in the previous fix round, leaving the PDF frame without an accessible name while its siblings keep theirs. Restore it (WCAG frame titles), assert it in the DOM test, and anchor the e2e on `iframe[title="report.pdf"]` instead of `iframe:not([title])`. * fix(projects): round-3 review — report the sweep's late finish, not a phantom cancel `Task.cancel()` returns False when the sweep already finished inside the window between the deadline firing and the cancel, so the shutdown log claimed a cancellation that never happened. Branch on that outcome: the warning stays for a real cancel, a late finish is logged at info, and both paths still reap the task before worker exit. * fix(projects): round-4 review — make Empty trash delete what it confirms `POST /api/trash/purge` only ran the retention sweep, and the sweep's candidate selection is age-gated, so a freshly trashed document survived "Empty trash" even though the confirmation promises that every listed document is permanently deleted. With one trashed row the route answered `{"purged": 0}` and left it in place; `GET /api/trash/documents` sweeps expired rows before listing, so the visible rows were normally ineligible for the action by construction. Empty trash now drives `purge_all_trashed`: the caller's trashed rows (`list_all_trashed`, no age filter) each go through the same guarded, row-locked `purge` as the single-document delete — bytes first, then the row, in one transaction — so a row restored mid-flight is skipped instead of force-deleted, and an unlink failure rolls that row back and answers 500 with a retryable message. Retention expiry stays where it was: the sweep's `purge_candidates` is now the only age-gated selection, and the lazy retention sweep still runs on the listing and at startup. Tests: the router suite replaces the retention-gated expectation with the reviewer's repro (fresh row purged, bytes unlinked, shelf and other users' trash untouched, a failing unlink stays retryable and 500); a blocking-I/O anchor drives the new entry point through the offload; the mocked e2e covers the action end to end; a new real-backend spec performs it against the real gateway and re-reads `GET /api/trash/documents`. README, API, ARCHITECTURE and the phase-2 design docs (en+zh) state the age-independent contract. |
||
|
alanhuangyoo
|
0f2813f6b9
|
fix(agents): keep queued guard warnings when a model call is retried (#5433)
* fix(agents): keep queued guard warnings when a model call is retried LoopDetectionMiddleware, TokenBudgetMiddleware and ToolProgressMiddleware pop their queued warning/hint before calling the model. When the call raises, LLMErrorHandlingMiddleware (outside them) retries by running their wrap_model_call again, and by then the queue is empty, so the retried request goes out without the warning. Loop detection and the token budget have already marked it as sent, so it is never queued again, and a loop runs on to the hard stop unwarned. Put the drained items back in front of the queue when the handler raises, then re-raise. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(agents): trim restored loop warnings from the tail and drop a dead helper _restore_pending_warnings put the restored warnings at the front and then trimmed the front, so if the cap ever fired it would drop exactly what it restored. Trim the tail, as tool progress does. _augment_request had no callers after the wrap_model_call change. Add the sync twin of the tool progress retry test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(agents): drop tool progress's unused _augment_request Its only remaining reference was a test name; the dedup that test checks lives in _inject_hints. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Hyeonsang Cho
|
f7f4a022e6
|
fix(agents): remove provider tool-call blocks when guards strip calls (#5447)
* fix(agents): remove provider tool-call blocks when guards strip calls Token-budget and loop-detection hard stops, subagent-limit truncation, and safety-finish-reason suppression removed calls from tool_calls and the raw additional_kwargs payload, but left the provider's own tool-call blocks in AIMessage.content. Provider adapters re-serialize those blocks: langchain_anthropic sends a tool_use block whose id is not in tool_calls, and the OpenAI Responses input builder sends every function_call block. ChatAnthropic stores any tool-calling response as a block list, so a guard firing on a Claude tool call always left a tool_use without a tool_result. A truncated subagent call failed the next model request of the same run; a hard stop was checkpointed under the same message id and failed every later turn of the thread. clone_ai_message_with_tool_calls now trims content tool-call blocks to the calls that remain on the message: tool_use and LangChain v1 tool_call/tool_call_chunk by id, Responses function_call and custom_tool_call by call_id (their id is the fc_ item id), Google GenAI function_call by id, and id-less blocks by name in order. Blocks for calls still on invalid_tool_calls stay, because DanglingToolCallMiddleware answers those calls with placeholder results. The token-budget and loop-detection hard stops now build their messages through the helper instead of their own copies, and ClarificationMiddleware drops its private filter, which matched Responses blocks by item id. * docs(changelog): reference #5447 in the orphaned tool-call block entry * fix(agents): skip id-matched calls in the id-less block budget The name budget for id-less content tool-call blocks counted every retained call, including calls whose own id-bearing block had already matched. In mixed-shape content, a retained call "a" with a function_call block carrying id "a" also let a same-named id-less block survive, leaving the unpaired block this helper exists to remove. Collect the retained ids that id-bearing blocks matched first, and build the name budget only from retained calls outside that set. Content with no id-bearing blocks keeps the full budget, so the Gemini path is unchanged. |
||
|
alanhuangyoo
|
740dbc4b39
|
fix(subagents): keep the subagent system prompt through context compaction (#5454)
* fix(subagents): keep the subagent system prompt through context compaction A subagent carries its whole system prompt (role, report contract, skills, deferred tools) as the leading SystemMessage in state, because its agent is built with system_prompt=None. Summarization only rescued tagged reminders and the latest user message, so the first compaction summarized the prompt away and every later model call in that subagent ran without its instructions. Rescue system messages as well. The lead agent's prompt is not in state, and its in-state SystemMessages are the tagged reminders already rescued, so the lead chain is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(compaction): clarify preservation contract and trim agent guidance --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
PeaceMaker-best
|
14c9d44440
|
feat(runtime): persist tool-progress phase transitions (#5214)
* feat(runtime): persist tool-progress phase transitions Record bounded warn, block, and recover decisions for lead and task subagent runs while preserving event-loop isolation, fail-open behavior, and concurrent transition order. * fix(runtime): trust server-owned tool progress attribution * fix(runtime): centralize trusted audit attribution * fix(runtime): preserve complete tool progress audit state * docs: trim tool progress guidance to pass size check * fix(runtime): fence subagent audit recorder loop * docs(readme): sync tool-progress event coverage Signed-off-by: PeaceMaker-best <221849497+PeaceMaker-best@users.noreply.github.com> --------- Signed-off-by: PeaceMaker-best <221849497+PeaceMaker-best@users.noreply.github.com> Co-authored-by: PeaceMaker-best <221849497+PeaceMaker-best@users.noreply.github.com> Co-authored-by: 嗜鵼 <hy2010hy2010@qq.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
yang rui
|
3dc895df4d
|
feat(models): pace shared RPM budgets before dispatch (#5432)
* feat(models): add shared RPM admission queues * fix(models): address admission pacing review feedback * docs: simplify request admission quick start guidance --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
alanhuangyoo
|
99902b7791
|
fix(agents): keep token budget signals for runs without a run_id (#5436)
* fix(agents): keep token budget signals for runs without a run_id #5410 moved every invocation without a non-empty string run_id onto str(id(runtime)). Two things break on that key: - SubagentExecutor passes the parent's run_id, None when the parent run has none (LangGraph Server, direct create_deerflow_agent callers), and reads the stop reason back with that None. The hard stop stored it under the id string, so a token-capped subagent reported a clean completion to the lead. - LangGraph gives each graph node its own Runtime wrapper, so the key changed between after_model and the next model call: the budget warning was never delivered, and each after_model counted every AIMessage in the thread. Key those invocations by Runtime.control, as LoopDetectionMiddleware does, release it in after_agent, and store the stop reason under the context run_id as given. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(agents): keep an active invocation's budget key when the anchor map is full The fallback anchor map was FIFO, so with 1000 run_id-less invocations on a shared instance an active one could lose its anchor mid-run and restart with a fresh budget. Move the anchor to the end on every lookup, as loop detection does, and note why execution_info.run_id is not consulted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Beautyl0ve
|
80f13935c2
|
feat(agents): allow custom agents to disable memory (#5167)
* feat(agents): allow custom agents to disable memory Signed-off-by: Beautyl0ve <74452755+Beautyl0ve@users.noreply.github.com> * fix(agents): honor memory opt-out during compaction Signed-off-by: Beautyl0ve <74452755+Beautyl0ve@users.noreply.github.com> * fix(runtime): preserve agent binding across state rewrites * fix(client): apply named-agent memory policy * fix(agents): address memory policy review feedback Signed-off-by: Beautyl0ve <74452755+Beautyl0ve@users.noreply.github.com> * fix(agents): address remaining memory opt-out reviews Signed-off-by: Beautyl0ve <74452755+Beautyl0ve@users.noreply.github.com> --------- Signed-off-by: Beautyl0ve <74452755+Beautyl0ve@users.noreply.github.com> Co-authored-by: PeaceMaker-best <221849497+PeaceMaker-best@users.noreply.github.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
哈基米
|
ba998a92ac
|
fix(mcp): treat a Human Input Card reply as the current user request (#5426)
McpRoutingMiddleware picked the latest user message with is_real_user_message, which rejects every hide_from_ui message. A Human Input Card reply is hidden but is still the user's current request, so when the routing keyword lived only in the clarification answer the deferred MCP tool was never auto-promoted and the model had to call tool_search by hand. Switch _latest_user_message to is_genuine_user_message, the same predicate summarization_middleware already uses for this reason (#5416). Fixes #5425 |
||
|
Totoro
|
533e30e7f2
|
[feat] add opt-in conversation reads for Gateway runs (#5399)
* feat: add scoped conversation reads to gateway runs * refactor: share conversation tool path and reuse parsed text --------- Co-authored-by: Totoro-qaq <279883115+Totoro-qaq@users.noreply.github.com> |
||
|
alanhuangyoo
|
f5cf25a8b6
|
fix(summarization): keep a Human Input Card reply as the current request (#5416)
Summarization rescues the latest user message by id so the current request survives compaction (#4882). It picked that message with is_real_user_message, which rejects every hide_from_ui HumanMessage, including Human Input Card replies. When a run starts from a card reply, the older visible request was kept and the user's answer was summarized away. Use is_genuine_user_message, which accepts hidden messages that carry a valid human_input_response and still skips other hidden injections. Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
alanhuangyoo
|
96d6ff3aca
|
fix(agents): keep the token budget across goal continuations of a run (#5410)
* fix(agents): keep the token budget across goal continuations of a run TokenBudgetMiddleware cleared its usage in after_agent, and before_agent marks every existing message as seen. A Gateway run re-enters the graph for hidden goal continuations under the same run_id, so each continuation started from zero and could spend another full budget, even after the user turn had already hit the hard stop. Keep the run's usage and warning state across graph entries, as LoopDetectionMiddleware does since #5344. Only the per-message seen map is dropped, and before_agent rebuilds it. Invocations without a run_id in the context still clear everything. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(agents): normalize token budget run identity and test cleanup --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Hyeonsang Cho
|
bbced51022
|
fix(gateway): close two input-sanitization bypasses (#5375)
* fix(gateway): reject forged framework-injection markers in run input
`is_genuine_user_message` treats `hide_from_ui` and a human `name="summary"`
as proof the framework authored a message, and `InputSanitizationMiddleware`
skips those — escaping a real reminder's blocks would corrupt trusted context.
Neither marker was server-owned, so an external caller could set either one and
place a raw `<system-reminder>` outside the user-input boundary markers, which
the lead-agent system prompt declares trusted internal framework data. The
`hide_from_ui` variant is also filtered out of the thread UI, so the forgery
was invisible where it landed.
Both markers are now stripped from untrusted input, on the run path and on the
thread-state mutation route that writes straight into a checkpoint. Framework
injection happens inside the graph and never crosses this boundary, so nothing
the framework does is affected, and `trusted_internal` callers (IM channels,
the MCP task-notification launcher) keep writing hidden messages.
HumanInputCard replies are the one legitimate external `hide_from_ui`: the
frontend sends it alongside a `human_input_response` payload, so a message
carrying a valid one keeps the marker. That buys no bypass — the predicate
already classifies those as genuine, so they stay sanitized.
The name check reuses the predicate's own `_SUMMARY_MESSAGE_NAME` rather than a
fourth copy of the literal, and matches by `isinstance` exactly as the predicate
does: `HumanMessageChunk` is a `HumanMessage` whose `type` is not `"human"`, so
a type-based check would leave that subclass's marker settable. `name` is only
reserved on human messages — on a ToolMessage it is the tool's own name.
Three tests in test_gateway_services.py and test_message_provenance.py asserted
that a caller-supplied `hide_from_ui` survives. That assumption was the bypass;
they now pin the opposite, with a genuinely caller-owned key kept alongside to
prove the stripper is surgical.
* fix(agents): sanitize every genuine user message, not only the newest
The input guardrail scanned backwards for the first genuine user message and
returned, so only the newest turn was ever sanitized. The transformation is
request-scoped (`wrap_model_call`, never written to state), so thread state
keeps the raw text: once a newer turn arrived, the previous turn's payload was
replayed to the model verbatim, outside the boundary markers the lead-agent
prompt declares trusted framework data. The guardrail therefore held for
exactly one model call.
Reaching it needed no forged metadata and no crafted request body — type the
payload in one turn, then send anything at all in the next. A single request
carrying two user messages did it in one shot, since every message but the last
was skipped.
`_process_request` now walks the whole list and `_sanitize_message` owns the
per-message work; every existing branch (the `original_user_content` split for
upload turns, the multimodal rfind fallback, the metadata repair) is unchanged.
Framework-injected messages stay excluded by `is_genuine_user_message`.
Unexpected errors now fail open per message rather than per request. Iterating
history widened the old blast radius: one unprocessable row would have dropped
sanitization for the whole request, handing an attacker the newest turn by
crafting an older one. `GraphBubbleUp` still propagates.
Side effect worth noting: each turn's rendering is now stable across model
calls. Previously a turn was wrapped on its own call and unwrapped on the next,
changing the prompt prefix behind the newest turn and defeating prompt caching.
test_only_processes_last_user_message pinned the old scope; it now pins that
every turn is processed, and keeps driving the `wrap_model_call` entry point.
* docs: record the message-metadata trust boundary and sanitization scope
`agents/middlewares/AGENTS.md` owns the depth for InputSanitizationMiddleware
and documented only the `original_user_content` half of its trust boundary. Left
alone it would teach an agent that `hide_from_ui` is caller-owned and that the
guardrail covers one turn — and the usual failure mode is an agent "restoring"
the behaviour it believes was lost. The entry now carries both markers, the
HumanInputCard exception, the whole-history scope, and the per-message fail-open
rule.
The note lives only there. The root and `backend/AGENTS.md` layers are
orientation that points at the module guides owning the depth, and
`backend/AGENTS.md` is inherited by every backend chain — prose added there
inflates more than twenty of them, and `scripts/check_agent_guidance.py` shows
the middlewares chain has about a kilobyte of room against its hard limit.
CHANGELOG.md and CHANGELOG_zh.md record it under Security, continuing the
existing prompt-injection lineage.
* fix(gateway): mark caller-hidden messages instead of stripping the marker
Review follow-up. Stripping a caller-supplied `hide_from_ui` closed the bypass
but broke three frontend senders that use the marker purely to keep a context
message out of the transcript: the quoted conversation context
(`buildHiddenConversationQuoteMessage`), the sidecar context prompt
(`buildHiddenSidecarContextMessage`), and the agent save command. None carries a
`human_input_response`, so the HumanInputCard carve-out did not cover them, and
nothing else hides them — `_is_branch_visible_message` and the frontend's
`isHiddenFromUIMessage` both key solely on `hide_from_ui`, and no backend reads
`conversation_quote_context` or `sidecar_context`. All three would have rendered
as user-visible chat bubbles.
The marker plays two roles and only one of them is a vulnerability. The security
requirement is that a caller-supplied marker cannot skip sanitization, not that
it cannot hide a message. So the roles are separated instead of the marker being
removed: the Gateway keeps it and stamps the server-owned `UNTRUSTED_INPUT_KEY`,
and the guardrail now asks `requires_input_sanitization` — the mark, else the
genuine-user test. Hidden stays hidden; untrusted content is sanitized either
way. The reserved `summary` name is handled the same way and no longer rewritten.
Marking rather than removing is also the safer shape in general: `hide_from_ui`
is read for presentation, journal persistence, memory filtering and IM outbound
as well, and this boundary should not silently change any of them.
`is_genuine_user_message` is deliberately left alone. `ToolReceiptMiddleware`
uses it for turn-boundary detection, where a caller's hidden context message must
keep counting as not user-authored; widening it there would move the ledger's
turn window. `requires_input_sanitization` sits beside it in `message_utils` so
the two questions can be compared.
The three tests that asserted a caller-supplied `hide_from_ui` is removed now
assert it survives and carries the mark — which restores the original intent of
the two provenance cases, whose comment already read "caller-owned keys must
survive".
* docs(gateway): rewrite normalize_input's docstring around the mark
Review follow-up. The paragraph still described the pre-4a3344f6 strip model and
contradicted both the implementation and the middlewares AGENTS.md paragraph
updated in that same commit: it called `hide_from_ui` server-owned, said
carrying it skips sanitization entirely, and repeated the premise this branch
disproved — that HumanInputCard replies are the only legitimate external use.
It now describes what the code does: the markers stay caller-owned and are
preserved because three frontend senders rely on `hide_from_ui` for hiding
alone, the message is stamped with `untrusted_input` instead, and
`requires_input_sanitization` sanitizes it anyway. `untrusted_input` joins the
server-owned inventory in the preceding paragraph, which is what makes the stamp
unforgeable and unclearable.
The three surrounding docstrings now also say that these functions mark as well
as strip; `_strip_external_message_metadata` had advertised only the removal,
leaving a reader no way to find the stamp from the name.
* fix(gateway): mark state writes whose message omits additional_kwargs
Review follow-up. The state-route half of the fix missed the most natural
request shape. `_strip_external_metadata_from_message_like` returned early when
`additional_kwargs` was absent or not a dict — there was nothing to strip — and
that early return also skipped the mark. A `POST /threads/{id}/state` body of
`{"values": {"messages": [{"role": "user", "name": "summary", "content":
"<system-reminder>…</system-reminder>"}]}}` therefore reached the checkpoint
unmarked. The messages reducer's `convert_to_messages` then supplies
`additional_kwargs={}`, so at model-call time `requires_input_sanitization` fell
back to `is_genuine_user_message`, which a `summary` name fails, and the forged
tag reached the model raw and outside the boundary markers.
A missing or non-dict `additional_kwargs` is now treated as empty for both the
strip and the mark. The identity return is kept for the case where nothing
changes, so an ordinary key-omitted message does not gain an empty dict just by
passing through. The run path was never affected: `normalize_input` coerces to
BaseMessage first, which always carries the dict.
Every existing state-write test supplied an `additional_kwargs` dict, which is
why this shape slipped through; the regression now covers it at the route and
end to end through the reducer into the guardrail.
While checking the neighbouring shapes, `_skips_input_guardrail` keyed off key
presence where `is_genuine_user_message` keys off truthiness, so
`hide_from_ui: False` — already covered without a mark — would have been
stamped. It now mirrors the predicate exactly, as its docstring claimed.
* docs(middlewares): compress the sanitization note to fit the guidance chain
main's growth left the middlewares AGENTS.md chain 84 bytes under its hard
limit, and the fuller wording did not fit. The load-bearing facts stay — the
markers are marked rather than stripped, and the scan covers every turn — since
those are the two an agent editing this middleware could otherwise get wrong.
The full model lives in the normalize_input, _mark_untrusted_framework_markers
and requires_input_sanitization docstrings.
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
|
||
|
wd_pan
|
5d86ce345c
|
feat(memory): add deterministic near-duplicate fact gate (#5254)
* feat(memory): add deterministic near-duplicate fact gate Add an opt-in write-side gate for DeerMem (issue #5252): a proposed NEW fact whose bounded token-Jaccard similarity to an existing fact in the same user/agent scope AND category reaches fact_dedup_similarity_threshold merges into that fact instead of being appended — the existing id/content/createdAt are kept, confidence is raised to the maximum, and the source is refreshed, with one facts_merged_dedup metric increment recording the merge. Defaults preserve the legacy behavior exactly; targeted updates by fact id and the exact-content key check are untouched. Companion write-side step to the read-side relevance/diversity work in #5251. Refs #5252 Signed-off-by: pwd11 <fvdsrc@163.com> * fix(memory): preserve corrections during fact deduplication Signed-off-by: pwd11 <fvdsrc@163.com> --------- Signed-off-by: pwd11 <fvdsrc@163.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Jun
|
dfc8e72428
|
fix(agents): make read-before-write cancellation lock-safe (#5395)
* test(agents): expose read-before-write cancellation races * fix(agents): make read-before-write cancellation lock-safe * test(agents): fix cancellation test import order * test(agents): apply cancellation test formatting * fix(agents): preserve first cancellation when worker task cancels * test(agents): cover cancelled worker preservation * fix(agents): preserve cancellation over worker errors * test(agents): cover worker error after cancellation |
||
|
Nan Gao
|
cf556fa9d4
|
feat(agents): elide superseded write_file payloads from model-bound requests (#5374)
* feat(agents): elide superseded write_file payloads from model-bound requests Step 2 of #5328. After a successful write_file the file on disk is the source of truth, and the read-before-write gate forces a read_file before the next modification of that path, so once a later successful read or write of the same path exists the historical `content` argument is redundant with it. Long report-writing runs (append-in-chunks) therefore carried every section twice, once as the write argument and once as the following read output, until summarization compacted the whole turn. - ToolOutputBudgetMiddleware's model-call hooks now replace such superseded content with a short deterministic placeholder pointing at read_file, in the model-bound request only: state["messages"], checkpoints, receipts, loop detection, and the run journal keep the original arguments, and nothing is externalized to disk. The newest `keep_recent_writes` successful writes (default 1) always stay visible; str_replace payloads are never touched; a same-turn read never supersedes (parallel calls run in no fixed order); only results stamped deerflow_tool_meta.status == "success" count, so failed, gate-blocked, partial, or unstamped writes are never candidates. - New `tool_output.elide_superseded_writes` (default on), `tool_output.superseded_write_min_chars` (default 2000), and `tool_output.keep_recent_writes` (default 1); config_version 41 -> 42 in config.example.yaml and the Helm chart. - The per-occurrence call/result pairing the gate introduced in #5329 moves into the shared `tool_call_args.pair_tool_call_results` helper so both policies pair the same way; the gate now uses it. * fix(agents): scope tool-call result pairing to the issuing turn Review finding on #5374 (P2): pair_tool_call_results consumed results from a history-wide per-id queue, so an interrupted write_file with no result whose tool-call id a later turn reused inherited that later call's success. With the default elision the unconfirmed draft was then replaced by a placeholder claiming the write succeeded, and the gate's blocked-call pairing had the mirror-image hole. Pair results the way DanglingToolCallMiddleware does: walk in document order, open each AIMessage's calls, and let a ToolMessage answer only a still-open call of the most recent preceding AIMessage. A result never answers a call from an earlier turn, so the interrupted call stays unanswered (never a candidate, never labeled blocked) and stray or duplicate results are ignored. Regressions cover the helper, the superseded-write policy, and the gate. * fix(agents): never rewrite tool-call ids duplicated within one AIMessage Review finding on #5374 (P2): the policies select calls per occurrence, but every provider surface is addressed by tool-call id, so when a malformed provider payload repeats an id inside one assistant turn the rewriter could only replace all of its occurrences at once. A failed write_file sibling then took on the superseded successful call's path and elided content and was presented as a success; the gate's blocked-call elision had the mirror-image hole (a successful sibling rewritten into the blocked call). rewrite_messages_tool_call_args now never offers an id that repeats within its message to the selector and leaves those calls untouched on every surface. Both policies are covered by the shared helper; regressions cover the helper, the superseded-write policy, and the gate. * fix(agents): skip unhashable tool-call ids in the duplicate-id guard Review finding on #5374 (round 3): _duplicated_call_ids fed every id into a Counter before the string guard, so a list or dict id from a malformed provider payload raised TypeError out of wrap_model_call and failed the whole model call whenever the history also held a rewrite candidate. The pre-PR loop and pair_tool_call_results skip such ids; only this helper regressed. Count non-empty string ids only, and pin it with regressions for the helper, the superseded-write policy, and the gate. * docs(agents): keep middleware guidance within size limit --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Wenchao An
|
1b76ab9060
|
feat: add opt-in task notes and compacted history recall (#5382)
* feat: add opt-in task notes and compacted history recall * fix: validate task continuity state and preserve user answers Honor explicit opt-out, preserve clarification replies and capture failure statuses, validate notebook writes, and clear branch archive references. Update the config version and audit optional LLM credentials, with regression and integration evidence. * fix: align Helm config version with task continuity schema * fix: preserve mixed task history and declare continuity policies * fix: recover malformed history and evict archives atomically |
||
|
Wenchao An
|
6d5d7bb1d5
|
feat(subagents): add opt-in parent context snapshots (#5367)
* feat(subagents): add opt-in parent context snapshots * test(subagents): package synthetic snapshot evaluation * fix(subagents): preserve output text and defer snapshot capture * docs(subagents): keep snapshot guidance within chain budget * fix(subagents): omit unpaired tool calls from snapshots * fix(subagents): safely omit unserializable snapshot media |
||
|
Hyeonsang Cho
|
bc4a33aba7
|
fix(skills): stop persisting resolved secrets when toggling skills (#5357)
Toggling a skill wrote resolved secrets into extensions_config.json. The Gateway skill toggle and DeerFlowClient.update_skill loaded the file with ExtensionsConfig.from_file(), which replaces every "$VAR" string with the environment value (and an unset variable with ""), then serialized that model back through to_file_dict(). A "$GITHUB_TOKEN" reference was persisted as the plaintext token, and an unset reference was erased for good. DeerFlowClient.update_mcp_config had the same flaw for every key other than mcpServers. Every writer now does a raw read-modify-write, the way the MCP router already did: read_raw_extensions_config reads the on-disk JSON, set_raw_skill_enabled changes only the target entry, and validate_raw_extensions_config checks the candidate the way the runtime will load it before the atomic write. When the file does not exist yet, the Gateway seeds it with the cached skill states only, never the resolved cached model. The MCP router's raw loader and candidate validation delegate to the same helpers, so the rule lives in one place, and to_file_dict() is removed so the unsafe serialization has no entry point left. Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
wutongyuonce
|
4470932118
|
feat(extensions): allow constructor kwargs on config-declared middlewares (#5312)
* feat(extensions): allow constructor kwargs on config-declared middlewares
extensions.middlewares entries may be a class-path string or {class, kwargs}.
String entries keep the zero-argument constructor. Unknown fields and blank
class paths fail at config validation. Constructor errors still fail at
agent creation.
Fixes #5311
* fix(extensions): coerce middleware kwargs to JSON types
YAML timestamps became datetime objects while JSON kept strings, so
constructors and to_file_dict() json.dump saw different types. Validate
kwargs as JSON types at config load, stringify dates, reject NaN and
other non-JSON values, and cover the raw-dict loader branch.
* style(extensions): wrap middleware Field description for ruff E501
make lint failed: the middlewares description was 289 chars (limit 240).
Wrap it and run ruff format on the two files this PR last touched.
* docs: compact configured middleware guidance to satisfy size limit
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
|
||
|
qian
|
9f4a7823e2
|
feat(title): use filenames for attachment-only conversations (#5304)
* feat(title): use filenames for attachment-only conversations * docs: trim upload guidance to fit inherited size budget * fix(title): bound attachment-only fallback titles --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Nan Gao
|
3f0b6ecc81
|
feat(agents): elide blocked write payloads from model-bound requests (#5329)
* feat(agents): elide blocked write payloads from model-bound requests A write_file / str_replace call rejected by the read-before-write gate never runs, yet its payload (up to 80 KB for a non-append write, unbounded for append) stayed verbatim in every later model request: nothing in the chain rewrites AIMessage tool-call arguments, and ToolOutputBudgetMiddleware only budgets ToolMessage output. The gate demands a re-read plus a fresh call, so the model re-emits the content anyway and the original is pure dead weight. - ReadBeforeWriteMiddleware stamps `deerflow_write_block` on the blocked ToolMessage and, in wrap_model_call, replaces the paired call's payload fields (content / old_str / new_str) with a short deterministic placeholder in the model-bound request only. state["messages"], receipts, loop detection, and the run journal keep the original arguments; nothing is externalized to disk, since a file reference to content the model must re-derive after reading the target would only invite bypassing the gate. - New `tool_call_args` helper rewrites every provider surface together (structured tool_calls, raw additional_kwargs.tool_calls, tool_use content blocks, tool_call_chunks) so strict providers never see them disagree; the gate only supplies the policy (which calls, what placeholder). - `read_before_write.elide_blocked_payloads` (default on) and `read_before_write.elide_min_chars` (default 2000) configure it; the runtime builder passes the config through and the middleware declares it via release_policy_parameters. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * docs(agents): condense middleware guide entry 11 to fit the guidance budget The agent-guidance CI check failed: the effective AGENTS.md chain for agents/middlewares was 99673 bytes against a 98304-byte hard limit. The chain already sat at 98459 on main, so the ReadBeforeWrite entry could not grow. Rewrite entry 11 so it states the same facts (gate, lock scope, fail-open, authorization scope, blocked-payload elision, shared tool_call_args helper) in 1229 bytes instead of 2640; the chain is now 98262 bytes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(config): bump config_version for the read_before_write elision keys Review follow-ups on #5329: - `read_before_write.elide_blocked_payloads` / `elide_min_chars` are new user-settable YAML keys, i.e. a config schema change, so bump `config_version` 40 -> 41 in config.example.yaml; without it an existing config.yaml gets no outdated-config warning and `make config-upgrade` has nothing to signal. - Say in the `elide_min_chars` description (and the example comment) that the threshold and the placeholder's size figure are Python character counts, not tokens: the same value spans roughly 3-4x in real context cost between ASCII and CJK text. - The builder wiring test now asserts only the wired `elide_min_chars` value instead of the whole `ReadBeforeWriteConfig` dump, so future knobs do not have to edit an unrelated test. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * chore(helm): bump chart config_version to 41 validate-chart's config_version drift check failed after config.example.yaml moved to 41 in ef9ee267. Bare bump of the chart's embedded `config:` block and the README example; the chart does not mirror the read_before_write section, so no field changes are needed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(agents): rewrite Responses and v1 content-block arguments too Review finding on #5329 (P2): the content rewriter only handled Anthropic `tool_use` blocks. With `use_responses_api=true` and `output_version='responses/v1'`, AIMessage.content carries `function_call` blocks whose `arguments` still hold the full write payload, and langchain_openai's Responses input builder emits that block instead of the rewritten structured call whose `call_id` it already carries. Standard `v1` `tool_call` blocks likewise keep `extras.arguments`, which the v1->Responses translator prefers over the structured args. So the blocked payload was still sent on every later Responses API request. `tool_call_args` now rewrites every content dialect that carries its own copy of the arguments: Anthropic `tool_use` (input, drop partial_json), Responses `function_call` (arguments, matched by call_id, `fc_...` item id and status preserved), and v1 `tool_call` / `tool_call_chunk` (args plus `extras.arguments`). Tests assert against the real adapter serializers: `_construct_responses_api_input` for responses/v1, v1, and v0 messages, `_convert_message_to_dict` for chat completions, and Anthropic `_format_messages` for native and v1 content, plus an end-to-end probe through the gate's wrap_model_call. * fix(agents): pair blocked writes per call occurrence and defeat Responses chaining Two review findings on #5329: - Tool-call ids may repeat across assistant turns (DanglingToolCallMiddleware pairs them with per-id queues). The gate matched blocked results against a history-wide id set, so a successful write sharing an id with a later (or earlier) blocked one also lost its payload and was labelled as blocked. `_blocked_call_occurrences` now pairs ToolMessages with call occurrences the same FIFO-per-id way and the selector keys on (message, call id). - With `use_previous_response_id`, the OpenAI adapter sends only the messages after the last AIMessage carrying a `resp_` response id and lets the server rebuild the rest from its stored copy, which still holds the original arguments and cannot be edited; every later response chains back to it. `rewrite_messages_tool_call_args` now drops every `resp_` id from the model-bound copy whenever it rewrote anything, so the adapter replays the full rewritten history (the `use_previous_response_id=False` request shape). OpenAI bills chained input tokens as input either way, so replay costs no more; the state keeps its ids. Tests cover success-before-block and block-before-success histories through the Chat Completions serializer, and chaining through `ChatOpenAI._get_request_payload` with `use_previous_response_id=True`: unrewritten history chains and omits the call, rewritten history is replayed with the placeholder and no `previous_response_id`. --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> |
||
|
PeaceMaker-best
|
36ce7590b7
|
fix(agents): isolate loop detection state by run (#5344)
* fix(agents): scope loop detection state per run * fix(agents): harden loop scope fallback * docs: move loop lifecycle detail out of inherited guidance --------- Co-authored-by: PeaceMaker-best <221849497+PeaceMaker-best@users.noreply.github.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Jun
|
3a6e681dee
|
fix(view-image): read active sandbox images from sandbox (#5306)
* fix(view-image): read remote sandbox images from sandbox * docs(tools): clarify view_image sandbox behavior * fix(view-image): address sandbox lifecycle review * fix(view-image): preserve image provenance across sandbox replacement * fix(view-image): address provider recovery review * fix(view-image): drain cancelled tool reads |
||
|
kbsama
|
3e1349576e
|
fix(agents): filter assembly descriptor subagent policy by allowed_subagents (#5205) (#5262)
* fix(agents): filter assembly descriptor subagent policy by allowed_subagents (#5205) * test(agents): ensure non-vacuous subagent catalog in assembly descriptor test --------- Co-authored-by: 1747687484-collab <229902011+1747687484-collab@users.noreply.github.com> |
||
|
Wenchao An
|
ca23703ef0
|
fix(subagents): preserve actionable acceptance gaps after compaction (#5287)
* fix: preserve actionable subagent acceptance gaps Distinguish completed execution from acceptance in delegation guidance. Retain bounded unmet and unverified criteria after compaction and guide the lead to address remaining work within its budget. * docs: keep acceptance guidance within instruction budget |
||
|
Sami Belhareth
|
f8f6cde23f
|
fix: preserve assistant/tool history in compaction summaries (#5248)
* fix: preserve bounded assistant and tool input during compaction * fix: retain recent fallback summary input and clarify budget * fix: preserve recent content in mixed-history summary fallback * docs: trim middleware guidance to pass size check --------- Co-authored-by: Sami Belhareth <6599699+belharethsami@users.noreply.github.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
PeaceMaker-best
|
e7c059d8d4
|
fix(agents): prioritize loop hard stops across tool batches (#5245)
* fix(agents): prioritize loop hard stops across tool batches Scan an admitted multi-tool response completely before selecting a soft warning, so any configured hard limit can reject the whole batch. Preserve warning priority and sliding-window accounting. Add counter-level plus sync and async compiled-agent regressions proving rejected tools are not executed. Fixes bytedance/deer-flow#5243. AI-assisted implementation and tests. * fix(agents): rearm loop warnings after cross-tool eviction When another tool evicts an older tool below its frequency warning threshold, clear the older suppression mark so a later burst can warn again. Add the cross-tool sliding-window regression from the final boundary review. AI-assisted implementation and tests. * test(agents): cover override-aware loop warning rearm Cache the default frequency thresholds for sliding-window eviction and verify that an evicted tool uses its configured override when warning eligibility is rearmed. Document that simultaneous frequency warnings preserve legacy first-crossing selection while hard stops remain batch-severity-first. Addresses review on #5245. AI-assisted implementation and tests. --------- Co-authored-by: PeaceMaker-best <221849497+PeaceMaker-best@users.noreply.github.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
早上肚子疼
|
383263bd34
|
fix(llm): release owned recovery probe on cancellation (#5197)
* fix(llm): release owned recovery probe on cancellation * docs: keep middleware guidance within chain budget --------- Co-authored-by: zaoshangduziteng <309590849+zaoshangduziteng@users.noreply.github.com> |
||
|
Beautyl0ve
|
3bccd1474f
|
fix(client): scope embedded agent reuse by effective user (#5206)
Signed-off-by: Beautyl0ve <74452755+Beautyl0ve@users.noreply.github.com> |
||
|
Hao Zhe
|
ec274bdedb
|
fix(memory): enforce backend read failure policy (#4726)
* fix(memory): enforce backend read failure policy * fix(memory): harden failure policy handling * fix(memory): narrow strict read handling * fix(memory): keep timeout handling off saturated executor * fix(memory): preserve legacy fail-closed timeouts |
||
|
PeaceMaker-best
|
3c36217a51
|
feat(observability): persist deferred tool promotions (#5183)
* feat(observability): persist deferred tool promotions Signed-off-by: PeaceMaker-best <221849497+PeaceMaker-best@users.noreply.github.com> * fix(ci): trim agent guidance chain Signed-off-by: PeaceMaker-best <221849497+PeaceMaker-best@users.noreply.github.com> --------- Signed-off-by: PeaceMaker-best <221849497+PeaceMaker-best@users.noreply.github.com> Co-authored-by: PeaceMaker-best <221849497+PeaceMaker-best@users.noreply.github.com> |
||
|
Michael
|
eebe909ebd
|
fix(agents): make the injected current-date timezone configurable (#5154)
* fix(agents): make the injected current-date timezone configurable ## Why The date reminder injected into the lead and subagent prompts (DynamicContextMiddleware / SubagentDateContextMiddleware) was formatted with the server's local wall clock. DeerFlow containers default to UTC, so a user in Asia/Shanghai chatting in the 00:00-08:00 window was told that 'today' is the previous day - the model then reasons, plans, and date-stamps against the wrong day. ## What changed - _format_current_date() now reads the optional DEER_FLOW_DATE_TIMEZONE env var (IANA name, e.g. Asia/Shanghai) and renders the date in that zone. - Unset = unchanged server-local behavior; invalid names log a warning and fall back to server-local. - Documented the knob in config.example.yaml, the module docstring, and the DynamicContext entry in agents/middlewares/AGENTS.md. ## Surface area - [x] Agents / LangGraph - prompt-layer date context only; message shape and midnight-update behavior unchanged - [ ] Frontend UI / Backend API / Sandbox / Skills / Dependencies - [x] Default behavior change (opt-in via env var - no behavior change unless set) ## Bug fix verification - New tests: test_format_current_date_honors_configured_timezone (UTC 20:30 -> 2026-09-03 in Asia/Shanghai), test_format_current_date_defaults_to_server_local_without_env, test_format_current_date_invalid_timezone_falls_back. - Existing mocked-datetime tests pass unchanged (no env -> datetime.now() path). ## Validation - cd backend && python -m pytest tests/test_dynamic_context_middleware.py: 31 passed. - blocking_io/test_dynamic_context_middleware.py: 2 pre-existing abefore_agent failures reproduce identically on clean main (blockbuster os.listdir detection on this host); the other 2 pass. - ruff format + ruff check clean. ## AI assistance **Tool(s) used:** Codex (coding agent) **How you used it:** analysis, implementation, and regression tests produced with AI assistance; reviewed before commit. - [ ] I've read and understand every line of this change and take responsibility for it — it's not unreviewed AI output. * fix(agents): avoid passing tz to datetime.now when no timezone is configured CI (backend-unit-tests shard 2) failed in test_tool_error_handling_middleware.py::test_subagent_chain_injects_date_without_memory_and_coalesces_for_strict_provider because its _FrozenDateTime.now() subclass override accepts no arguments, while _format_current_date() called datetime.now(None) even when DEER_FLOW_DATE_TIMEZONE was unset. - _format_current_date() now calls datetime.now() with no arguments unless a timezone is actually configured, preserving the exact legacy call shape for every datetime-subclass test fake. - The configured-zone path still calls datetime.now(tz) and converts via astimezone(tz). - Updated the no-env unit test to assert datetime.now() is called without arguments. Validation: python -m pytest tests/test_dynamic_context_middleware.py + the previously failing strict-provider test: 32 passed. ruff clean. * fix(agents): declare the effective current-date timezone in the assembly descriptor ## Why Maintainer review on the DEER_FLOW_DATE_TIMEZONE change (#5154): the knob is prompt-affecting, yet both DynamicContextMiddleware and SubagentDateContextMiddleware were invisible to the agent assembly descriptor - describe_middleware() fell back to {"probed": true} for unset, UTC, and Asia/Shanghai alike, so deployments that inject different dates shared one assembly fingerprint and release observers could not distinguish or audit the behavior change. ## What changed - Both middlewares now implement release_policy_parameters() -> dict[str, object], declaring {"current_date_timezone": <name>} as required by the module's middleware self-description contract. - The declared value is the normalized effective zone: a configured, valid DEER_FLOW_DATE_TIMEZONE is reported by its IANA key (ZoneInfo.key); otherwise the server-local zone is resolved to its IANA key when the platform exposes one and to its tzname label otherwise (fixed-offset hosts), with "UTC" as the final fallback. - Added both middlewares to _MIDDLEWARE_DECLARATIONS in backend/tests/test_middleware_release_policy.py so the existence check and the construct-and-canonical-hash check cover them. ## Verification - New tests: test_date_middlewares_declare_configured_timezone (Asia/Shanghai), test_date_middlewares_declare_utc_timezone, plus resolved-server-local assertions for the unset and invalid-env paths; both middlewares agree in every case. - cd backend && python -m pytest tests/test_dynamic_context_middleware.py tests/test_middleware_release_policy.py: 70 passed. - Regression spot-check: tests/test_agent_assembly_descriptor.py, tests/test_tool_error_handling_middleware.py, tests/test_system_message_coalescing_middleware.py: 102 passed. - ruff check + ruff format clean. ## AI assistance **Tool(s) used:** Codex (coding agent) **How you used it:** analysis, implementation, and regression tests produced with AI assistance; reviewed before commit. - [ ] I've read and understand every line of this change and take responsibility for it — it's not unreviewed AI output. * fix(agents): stabilize the declared date timezone and simplify the formatting path ## Why Follow-up review on #5154 (willem-bd). The release-policy declaration added in 884cec4b resolved the observability gap but pinned far less identity than its docstrings claimed, and the formatting path carried a production no-op. ## What changed - The declared label is now stable and unambiguous: a configured, valid DEER_FLOW_DATE_TIMEZONE is reported by its IANA key; without one, the server-local zone is resolved to a real IANA key from the TZ env var or the /etc/localtime symlink (Linux/macOS); when no key is recoverable (Windows, stripped containers) the declaration falls back to a stable `server-local(+-HH:MM)` sentinel carrying the current UTC offset. It never reports a bare abbreviation - datetime.now().astimezone() yields only a fixed-offset timezone whose tzname (e.g. CST, EST/EDT, CET/CEST) is ambiguous or DST-churns, which the assembly descriptor docstring says must not happen. - Dropped the redundant astimezone(tz) in _format_current_date(): datetime.now(tz) already returns the instant expressed in tz. The configured-zone test now fakes datetime.now(tz) semantics (the fixed instant converted into the requested zone) instead of relying on that conversion. - Documented why the knob is an env var, not a config-schema field: it is read at runtime by both date-context middlewares so an operator can point a container at another zone without mounting a config.yaml (module docstring + config.example.yaml note). - AGENTS.md: fixed the glued DynamicContext sentence (missing separator). - Added tzdata>=2025.1 to the harness runtime dependencies (with uv.lock) so ZoneInfo works on stripped containers / Windows without an OS zone database. ## Verification - New tests: test_server_local_timezone_name_reads_tz_env, test_effective_timezone_sentinel_uses_offset_when_local_zone_is_not_resolvable; reworked test_format_current_date_honors_configured_timezone to exercise the real datetime.now(tz) path. - cd backend && python -m pytest tests/test_dynamic_context_middleware.py tests/test_middleware_release_policy.py tests/test_agent_assembly_descriptor.py tests/test_tool_error_handling_middleware.py: 140 passed. - ruff check + ruff format clean. ## AI assistance **Tool(s) used:** Codex (coding agent) **How you used it:** analysis, implementation, and regression tests produced with AI assistance; reviewed before commit. - [ ] I've read and understand every line of this change and take responsibility for it — it's not unreviewed AI output. * fix(agents): offload subagent date injection off the event loop ## Why Follow-up review on #5154 (willem-bd, P2): SubagentDateContextMiddleware.abefore_agent() called _inject() directly, so enabling DEER_FLOW_DATE_TIMEZONE could synchronously read the OS timezone database (or the tzdata wheel) on a cold cache - filesystem work on the async subagent execution path whenever no assembly observer resolved the zone first. ## What changed - SubagentDateContextMiddleware.abefore_agent() now offloads the injection via asyncio.to_thread with the same bounded timeout DynamicContextMiddleware uses (issue #3402); on timeout it logs and skips the date update for that run instead of blocking the loop. - Narrowed the exception handling in _date_timezone() and the TZ-env branch of _server_local_timezone_name() to configuration-shaped failures (ZoneInfoNotFoundError / ValueError / OSError). Previously a blanket `except Exception` also swallowed BlockingError raised by the blocking-I/O regression gate, mislabeling a loop-blocking call as an invalid timezone and silently degrading to server-local - which made the new regression anchor useless. Other exceptions now propagate. ## Verification - New blocking-I/O regression anchor (backend/tests/blocking_io/test_subagent_date_context_middleware.py): drives a real create_agent graph under the strict Blockbuster gate with the knob enabled and asserts the date reminder is injected. Verified it fails (BlockingError) when the offload is reverted and passes with it in place. - python -m pytest tests/blocking_io/test_subagent_date_context_middleware.py: 1 passed. The two pre-existing os.listdir failures in tests/blocking_io/test_dynamic_context_middleware.py reproduce unchanged on this host (same as clean main). - python -m pytest tests/test_dynamic_context_middleware.py tests/test_middleware_release_policy.py tests/test_tool_error_handling_middleware.py tests/test_agent_assembly_descriptor.py: 139 passed; the single ToolReceiptMiddleware-ordering failure reproduces with the change stashed (local extensions registry, unrelated to this PR). - ruff check + ruff format clean. ## AI assistance **Tool(s) used:** Codex (coding agent) **How you used it:** analysis, implementation, and regression tests produced with AI assistance; reviewed before commit. - [ ] I've read and understand every line of this change and take responsibility for it — it's not unreviewed AI output. * fix(agents): read the direct /etc/localtime symlink target for the zone key ## Why Follow-up review on #5154 (willem-bd, P2): on macOS, /etc/localtime commonly points to /var/db/timezone/zoneinfo/<zone>, but Path.resolve() follows that directory's own symlink and yields a versioned path such as /private/var/db/timezone/tz/2026c.1.0/zoneinfo/Asia/Shanghai, which matched no configured prefix. The server-local resolution then returned None and the assembly descriptor fell back to a server-local(+HH:MM) sentinel even though the IANA key was available - conflating zones that share an offset and making DST-based fingerprints unstable. ## What changed - _server_local_timezone_name() now reads the direct symlink target via os.readlink("/etc/localtime") instead of Path.resolve(), so macOS' unversioned zoneinfo path is seen as-is and its IANA key is preserved. - The zone key is taken from whatever follows the last "/zoneinfo/" segment, which also handles Apple's canonical versioned path when a direct target already carries it, and relative targets are normalized against /etc. - Removed the now-unused Path import and the fixed zoneinfo prefix tuple. ## Verification - New tests: test_server_local_timezone_name_reads_direct_macos_symlink_target, test_server_local_timezone_name_reads_apple_versioned_symlink_target, and test_server_local_timezone_name_normalizes_relative_symlink_target. - python -m pytest tests/test_dynamic_context_middleware.py tests/test_middleware_release_policy.py tests/test_agent_assembly_descriptor.py: 105 passed (75 after re-running the first two on the merged main). The blocking subagent anchor still passes; the two pre-existing os.listdir blocking failures on this host are unchanged. - ruff check + ruff format clean. ## AI assistance **Tool(s) used:** Codex (coding agent) **How you used it:** analysis, implementation, and regression tests produced with AI assistance; reviewed before commit. - [ ] I've read and understand every line of this change and take responsibility for it — it's not unreviewed AI output. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
luo jiyin
|
e5977320a0
|
feat(sandbox): surface structured mount upload result on E2B sandbox (#4884)
* feat(e2b-sandbox): make mount upload deadline configurable Replace the hardcoded 120-second mount upload deadline with a configurable `mount_upload_deadline_seconds` key read from SandboxConfig (extra=allow). The value is validated: zero and negative inputs are clamped to 1 second. Omitting the key preserves the existing 120-second default. This addresses the follow-up from PR #4842 review: operators with large mounts or slow networks can now size the deadline to their deployment without changing code. * fix(e2b-sandbox): address review feedback on configurable deadline - Remove import-time default capture from _mount_deadline_reason() and _MountUploadBudget.deadline_seconds to prevent silent drift. - Add warning log when mount_upload_deadline_seconds is clamped to 1 (was silent before). - Update AGENTS.md E2B Mount Uploads section: deadline is now configurable, not fixed 120. - Add mount_upload_deadline_seconds to YAML examples in provider docstring and __init__.py. - Add config-path test that exercises SandboxConfig -> _load_config -> _apply_mounts end-to-end. * feat(e2b-sandbox): surface structured mount upload result on sandbox Introduce MountUploadResult dataclass and attach it to E2BSandbox.mount_upload_result after creation. This makes mount truncation observable in code without re-parsing Gateway logs. _apply_mounts() now returns MountUploadResult with truncated, reason, and upload totals. _create_sandbox() captures the result, stores it on the sandbox instance, and records it in a provider-level map so the result survives warm-pool reclaim and reconnect. MountUploadResult.truncated is True only when the upload pass was stopped early by a resource limit (deadline, file count cap, or byte budget). Individual mount failures (missing host path, SDK errors) are logged but do NOT set truncated. Tests cover: success totals, deadline truncation, file-count truncation, byte-budget truncation, non-limit failure not reported as truncation, missing host path not reported as truncation, create→sandbox wiring, and create→release→warm-pool→acquire result preservation. * fix(e2b-sandbox-provider): fix _mount_results lifecycle leak and review findings - Add _forget_mount_result() helper and call it at all terminal sandbox paths: _reuse_in_process_sandbox dead-evict, _reclaim_warm_pool_sandbox reconnect/dead/bootstrap/ownership/shutdown failure branches, _forget_local_sandbox, _kill_and_close. Prevents unbounded dict growth over a long-running Gateway process. - Make MountUploadResult @dataclass(frozen=True) to prevent silent mutation of the shared reference between provider map and sandbox attribute. - Move _mount_results insert under self._lock in _create_sandbox to match the read discipline in _register_connected_sandbox. - Guard _resolve_mount_upload_deadline against None (YAML explicit null) to avoid int(None) TypeError. - Add 5 regression tests covering each bypass path and the frozen invariant. * fix(e2b-sandbox-provider): add _forget_mount_result to _evict_oldest_warm branches Add _forget_mount_result() calls to all four terminal exit paths in the E2B _evict_oldest_warm override (reconnect failure, already-gone, kill failure, kill success). The peer-owned path already cleans up via _forget_local_sandbox. Add test_evict_oldest_warm_cleans_mount_result to pin the kill-success branch. * docs: reduce agent guidance size --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Tu Naichao
|
ae82f426bf
|
fix(summarization): stop fraction triggers from crashing the agent build (#4901)
* fix(summarization): resolve fraction triggers from declared context_window, degrade instead of crashing the agent build A fraction trigger/keep clause requires profile["max_input_tokens"], which any third-party OpenAI-compatible model lacks, so SummarizationMiddleware construction raised ValueError out of create_summarization_middleware and failed the whole agent build (#3103). - factory: translate a declared model context_window into the langchain profile (metadata-only, never reaches the provider payload); explicit caller/override profiles win - summarization factory: drop unusable fraction trigger clauses (absolute clauses survive), fall a fraction keep back to the messages default, and disable compaction with an actionable warning only when no usable trigger clause remains — the agent build never dies from summarization config - docs: config.example.yaml, ModelConfig.context_window, summarization.md * refactor(summarization): share the default keep constant with the fraction fallback The fraction-keep degradation fallback hardcoded ("messages", 20), duplicating SummarizationConfig.keep's default_factory literal. Move the value to a shared DEFAULT_KEEP constant so the two cannot drift apart. * fix(summarization): keep trigger-null + fraction-keep constructing after degradation A trigger of None with a fraction keep hit the all-clauses-dropped branch (has_usable_trigger=False) and disabled compaction, and the accompanying warning claimed configured triggers were all fraction-based when none were configured. Only report nothing-usable when trigger clauses actually existed; trigger:null keeps constructing the never-firing middleware with the degraded keep, matching its behavior outside the degradation path. * fix(summarization): address review — keep manual compaction, validate ContextSize, pin wiring Review follow-ups on #4901: - When every configured trigger is a dropped fraction clause, keep constructing the never-firing middleware (trigger=None) instead of returning None: manual /compact runs with force=True and never consults trigger clauses, so it must keep working for a profile-less model rather than reporting 'compaction is disabled'. The warning now says auto-compaction will not fire while manual compaction remains. - ContextSize gains a config-load validator: fraction values must be in (0,1] (a percent-style 80 instead of 0.8 previously produced a threshold the context could never reach — a silently inert trigger), absolute values must be positive. - New un-monkeypatched integration test pins the shipped wiring (context_window declared -> real factory attaches profile -> fraction clause survives -> middleware constructs), which the stubbed middleware-side tests and kwarg-capturing factory-side tests each stopped short of. - Docs (summarization.md + config.example.yaml) clarify that the fraction resolves against the summary/anchor model's context_window (summarization.model_name when set, else the run model), including the mismatch caveat for a larger-window summary model. * fix(summarization): reject non-finite ContextSize values at config load YAML .nan / .inf pass pydantic's float parsing, and nan <= 0 is False, so the positivity check alone let them through as dead thresholds (count >= nan is always False) — the same silent-inert-trigger class the range validator was added to close. Guard with math.isfinite first, consistent with the existing non-finite guards on mem0 timeout_seconds and poll_after_seconds. * fix(summarization): merge context_window into inferred profile, require whole message counts - construct the model first, then merge max_input_tokens into the provider-inferred langchain profile: passing profile= to the constructor replaced the whole inferred metadata (tool_calling, structured_output, io capabilities, output limits) with the single key. An explicitly configured profile is still never clobbered. - reject non-integral ContextSize values for type=messages at config load: langchain slices the message list with them, so a float index raised TypeError mid-compaction. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Otavio Rodrigues Santana
|
27cb73659d
|
fix(auth): correct OAuth conflict error message + validate multi-worker Postgres claim with real concurrency benchmark (#5026)
* fix(auth): correct OAuth uniqueness error and index parity on Postgres
create_user() caught any IntegrityError on commit and always reported it
as a duplicate email. The email pre-check already rules out a real email
collision in the common case, so any IntegrityError reaching that handler
is actually idx_users_oauth_identity firing instead -- confirmed against
both backends: SQLite reports "UNIQUE constraint failed:
users.oauth_provider, users.oauth_id", Postgres reports a
UniqueViolationError naming the same index. The caller saw "Email already
registered" for an OAuth account conflict, which is wrong and would send
API consumers debugging the wrong field.
Distinguish the two cases via a substring check on the driver error text
(both backends name the oauth columns) and raise an accurate message for
each.
Also add postgresql_where to the same index, alongside the existing
sqlite_where. This is not a correctness fix -- verified empirically that
Postgres already enforces the same practical uniqueness without it
(NULL is never equal to NULL in either backends unique index, so real
duplicate (provider, id) pairs are already rejected and NULL/NULL rows
are already unconstrained). postgresql_where makes the index genuinely
partial on Postgres too, matching the stated intent in the surrounding
comment and keeping the index smaller as the common case (plain-password
accounts, both columns NULL) accumulates.
* test(bench): add multi-process SQLite vs Postgres concurrency benchmark
CONFIGURATION.md documents that multi-worker deployments must use Postgres
because "SQLite silently ignores row-level locks", but nothing in the repo
exercised that claim against real separate worker processes -- the existing
checkpoint benchmarks (scripts/benchmark/checkpoint/) measure single-process
read/write latency, and the existing Postgres tests
(test_pg_schema_integration.py, test_multi_worker_postgres_gate.py) cover
schema placement and config validation, not throughput or lock behavior
under concurrent load.
run_concurrency_bench.py spawns N real OS processes (subprocess.Popen, not
asyncio tasks or threads within one process) against the shared users
table, mixing reads (get_user_by_email) and writes (create_user) at a
configurable ratio, and reports throughput, error counts by exception
type, and p50/p95/p99/max latency per run.
Measured locally (2/4/8/16 workers, 100 ops/worker, 70/30 read/write):
SQLite completed all operations with zero errors at every worker count
(busy_timeout absorbs contention rather than raising), but total
throughput stayed flat around 28-34 ops/s regardless of worker count, and
p99 latency grew from ~400ms at 2 workers to ~5.9s at 16, with a 22s max.
Postgres throughput scaled with worker count (41 to 66 ops/s) and p99
stayed under 500ms at every worker count tested. Raw JSON output from
both runs is available on request; exact numbers will vary by machine and
are not asserted in the test suite.
test_bench_concurrency.py unit-tests the pure aggregation logic
(percentile math, error grouping, crashed-worker handling) the same way
test_bench_checkpoint_channels.py does for the existing benchmarks --
fast, no DB required, not the full multi-process sweep in CI.
* fix(auth): inspect the driver exception for OAuth conflict detection
str(exc) embeds the full failed INSERT statement, whose column list
always names oauth_provider/oauth_id, so a substring check on it
misclassified every commit-time IntegrityError on the users table as
an OAuth conflict (reproduced on SQLite: a duplicate primary key with
a different email raised "OAuth account already linked: None/None").
_is_oauth_identity_violation now inspects exc.orig instead: constraint_name
on Postgres, both violated column names present (not a bare "oauth"
substring) on SQLite.
Also ships the alembic revision idx_users_oauth_identity's postgresql_where
predicate needed: 0001_baseline created it as a full index on Postgres,
and ORM metadata changes only affect fresh create_all databases, never an
already-versioned deployment.
Addresses review feedback from willem-bd.
* fix(bench): run the concurrency benchmark in an isolated schema and derive paths from the checkout
--pg-url accepted an arbitrary database URL while the code pinned
postgres_schema="public" and unconditionally ran DELETE FROM users --
against any non-disposable database that permanently destroyed every
auth account. Each run now generates a unique throwaway schema
(bench_<uuid>), points both the seeder and every worker subprocess at
it via postgres_schema, and drops only that schema (DROP SCHEMA ...
CASCADE) once the full worker-count sweep finishes.
Also stopped hard-coding /opt/deer-flow/backend as the checkout path
and .venv/bin/python3 as the interpreter: BACKEND_DIR is now derived
from Path(__file__), and workers are spawned with sys.executable (the
orchestrator's own interpreter) instead, so the documented
uv run python scripts/benchmark/concurrency/run_concurrency_bench.py
command works from any checkout.
Addresses review feedback from willem-bd.
* fix: shorten oauth-index revision id, repin migration-head assertions, fix bench read/write mix
- 0017_users_oauth_identity_partial_pg (36 chars) exceeded
alembic_version.version_num's VARCHAR(32) limit, which would fail
stamping/upgrading on both fresh and existing Postgres deployments.
Renamed to 0017_oauth_identity_pg_partial (30 chars).
- Repinned every test asserting 0016_subagent_batches as the migration
head (test_persistence_bootstrap[.py|_concurrency.py|_regression.py],
test_migration_0004/0007/0015) to the new 0017 revision id.
- worker.py's `(i % 100) < int(read_ratio * 100)` assumed n_ops >= 100;
at the documented default (50 ops/worker, 0.7 read ratio) it produced
either all-reads or all-writes, never the claimed mixed workload.
Replaced with read_count()/is_read_op(), which distribute an exact
round(n_ops * read_ratio) reads evenly across the sequence via modular
spacing, and added test_bench_worker.py covering the default values
plus small op counts.
* fix(bench): establish a real physical connection before timing ops
async with sf(): pass entered an empty AsyncSession without checking out
a physical connection -- SQLAlchemy stays lazy until the first statement
executes. That pushed connection-establishment cost onto each worker's
first timed operation instead of conn_time_s, and at 16 workers those 16
cold first-ops (1% of a 1600-op sample) could skew the reported p99.
Execute a real `SELECT 1` before starting the timer instead.
Verified with a real end-to-end run (uv sync + sqlite backend, 2
workers/10 ops, 0 errors) plus the full auth/bench/migration-bootstrap
suites (135 tests) and ruff check/format, all clean.
* fix(bench): synchronize workers before timing, fix percentile off-by-one
Two remaining measurement issues from review:
- run_workers() started the wall clock before spawning any worker, so
throughput/wall_time absorbed N processes' staggered Python-startup and
connection-establishment cost, and early workers could run ahead of ones
still starting. Workers now print READY right before their timed loop
and block on stdin for a GO signal; the orchestrator waits for every
READY, then starts the timer and releases all workers together.
- summarize()'s pct() used int(len(latencies) * p) directly as a
zero-based index -- a one-based-rank-as-index bug that put p95 and p99
at the same slot (the max) for any 20-or-fewer-sample run, and for the
documented 100-sample default. Now delegates to
checkpoint_bench_common.percentile(), the already-correct nearest-rank
implementation used elsewhere in the same benchmark family, instead of
a second, broken one.
Verified: 14/14 unit tests pass (2 new pinned-value regression tests for
the percentile bug, using the reviewer's own 20-sample repro), ruff
clean, and a real 2/4-worker SQLite multi-process smoke run completes
with distinct p95/p99/max latencies and no hang.
* fix(bench): absolute SQLite bench path, surface crash diagnostics, exit nonzero on failure; share OAuth index constant + cover Postgres branch
Three more findings from review at 5fd25a7:
- seed_baseline() cleaned an absolute .deer-flow/bench_data path but
handed DatabaseConfig a relative one, which resolves against the
CALLER's CWD -- not BACKEND_DIR. Invoking the documented command from
anywhere other than backend/ silently pointed the seeder and the
(cwd=BACKEND_DIR) workers at two different directories: workers crashed
with 'unable to open database file' while the run still printed a
well-formed summary and exited 0. Both seed_baseline() and worker.py's
make_session_factory() now use the same absolute path.
- Crashed workers' stderr was captured then discarded, and main() always
exited 0 -- an all-crashed sweep was indistinguishable from a real
(uneventful) measurement to anything checking the exit code or
--out. run_workers() now prints each crash immediately and tags it with
the real worker_id (previously always None); summarize() exposes
crashed_worker_errors alongside the existing crashed_workers count;
main() exits 1 via the new summary_indicates_failure() whenever any
sweep crashed or fell short of expected_total_ops.
- idx_users_oauth_identity was hardcoded separately in the ORM Index and
in _is_oauth_identity_violation's Postgres branch, with no test to
catch drift, and that branch had zero non-skipped coverage (its only
guard needs a live Postgres CI never configures). Exported
OAUTH_IDENTITY_INDEX_NAME from user/model.py as the shared source of
truth (migrations intentionally keep their own frozen literal, matching
every other revision in that package) and added stub-exception unit
tests pinning both the asyncpg constraint_name path and the sqlite
message-substring path, positive and negative.
Verified: 107 passed locally (auth + bench-unit suites), ruff clean, and
two real reproductions -- invoking run_concurrency_bench.py from a
scratch directory outside backend/ (the reviewer's exact repro) now
completes 8/8 ops with crashed_workers: 0 instead of crashing, and the
new crashed_worker_errors/exit-code logic is exercised directly by the
new unit tests against the real summarize()/summary_indicates_failure().
* fix(auth): attribute create_user IntegrityErrors to the right constraint
Two coupled review findings on the classification helpers:
P3 (fall-through) -- after ruling out the OAuth-identity index, create_user
raised "Email already registered: {email}" for every remaining
IntegrityError, including the duplicate-primary-key case the new
regression test exercises, whose address is not registered. Added
_is_email_violation() so the email message is used only for an actual
users.email collision that raced past the pre-check; anything else (in
practice a duplicate id) now raises a neutral
"User already exists (constraint: <name>)".
P2 (unreachable asyncpg branch) -- exc.orig is not the asyncpg error.
SQLAlchemy's asyncpg dialect re-raises its own DBAPI IntegrityError
(pgcode/sqlstate only) 'from' the real asyncpg error, so constraint_name
lives on exc.orig.__cause__. getattr(exc.orig, "constraint_name", None)
was always None on Postgres; the helpers only worked there by accident,
matching asyncpg's DETAIL line in the message fallback. Added
_driver_constraint_name() which walks orig then orig.__cause__, and the
stub tests now model that real shape (orig wrapper + __cause__) instead of
a constraint_name that no driver puts on orig directly.
Tests: 76 passed. New coverage for the email-race path, both new helpers
on each backend, the neutral fallback message, and the cause-chain walk.
* fix(bench): match app SQLite PRAGMAs in workers; fail a sweep on any op error
Two review follow-ups:
- worker.py opened its SQLite engine with only connect_args timeout=30.
synchronous and foreign_keys are per-connection PRAGMAs, so workers ran
at SQLite's synchronous=FULL / foreign_keys=OFF while a real Gateway
worker runs synchronous=NORMAL (persistence/engine.py::_enable_sqlite_wal)
-- an extra fsync per commit on the measured 30%-write path, overstating
SQLite's cost in the direction that flatters the "use Postgres"
conclusion. Added a connect listener applying the same four PRAGMAs, with
a test asserting synchronous/foreign_keys/journal_mode on a real worker
connection.
- summary_indicates_failure() only looked at crashes and the completed vs
expected op counts, so a sweep where every op completed but raised
(e.g. writes hitting OperationalError) passed as a clean measurement:
completed_ops == expected, 0 crashes. Added an "errors > 0" clause; the
error breakdown stays in the JSON, only the exit code changes. Test added.
test_bench_concurrency.py + test_bench_worker.py green (20), plus a real
2-worker sqlite smoke run (6/6 ops, 0 errors, exit 0).
* fix(auth): match the real email index name; only claim "exists" for uniqueness
Review follow-ups on the classification helpers:
- email is mapped_column(unique=True, index=True), which SQLAlchemy and
0001_baseline realise as a single UNIQUE INDEX (ix_users_email), not a
named UNIQUE constraint. _is_email_violation compared the driver
constraint name against "users_email_key", which Postgres never emits,
so that arm was dead on Postgres (SQLite matched via the message). Fixed
to ix_users_email.
- the residual IntegrityError fallback raised "User already exists" for
every remaining IntegrityError -- a NOT NULL / CHECK / foreign-key
violation is not a "user already exists" condition and is not part of
create_user's ValueError contract. Added _is_uniqueness_violation
(sqlstate 23505, or the SQLite "UNIQUE/PRIMARY KEY constraint failed"
message); only that raises the "already exists" ValueError, everything
else propagates unchanged.
- documented scripts/benchmark/concurrency/ in backend/AGENTS.md alongside
the other benchmark family.
Tests: 78 auth + 20 bench-unit pass, ruff clean. New coverage for
_is_uniqueness_violation on both backends and for a non-uniqueness
IntegrityError propagating out of create_user.
* fix(bench): don't pre-close worker stdin (breaks communicate); require --pg-url for postgres
* fix(bench): ruff format; time throughput on the op phase, not teardown
- lint-backend: ruff format the files touched in this PR.
- Throughput window (P2): the orchestrator sampled its wall clock after
every worker's communicate() returned, so it also covered each worker's
engine.dispose(), result serialization and stdout transfer. Each worker
now times just its operation phase (GO -> last op) and reports
ops_elapsed_s; summarize() uses max(ops_elapsed_s) over the workers -- all
released by the same GO -- as the throughput window (ops_window_s).
- Exercise migration 0018 (P2): test_user_oauth_partial_index.py goes
through bootstrap create_all(), which builds the partial index from ORM
metadata and never runs 0018.upgrade(). New Postgres-gated
test_migration_0018_oauth_identity_pg_partial.py alembic-upgrades to 0017
(full index), then 0018 (asserts the predicate appears), then downgrades
(asserts the full index is restored) and re-upgrades.
* docs(middlewares): tighten SandboxAudit and Clarification entries in AGENTS.md
PR #5134 grew agents/middlewares/AGENTS.md ~1.8 KB, pushing the effective
AGENTS.md chain for that directory over the 96 KiB hard limit once this
branch also documents scripts/benchmark/concurrency/ in backend/AGENTS.md.
Condense the two longest middleware entries (SandboxAuditMiddleware,
ClarificationMiddleware) without dropping any identifier, example, issue
reference, ordering constraint, or documented gap; chain back to ~96.8 KiB.
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
|
||
|
SPEC
|
822c7bca4b
|
fix(memory): cancel buffered extraction when agent is deleted or cleared (#5123)
* fix(memory): cancel buffered extraction when agent is deleted or cleared * fix(memory): cancel buffered work before agent delete Address review: cancel before/after delete to close the rmtree race, scope user_id=None cancels to the legacy root only, and import memory helpers at module scope. Signed-off-by: SPEC <zt1y17@soton.ac.uk> * fix(memory): close remaining cancel races from review Post-clear cancel, legacy-only all_agents scope, always cancel even when memory is disabled, and fold cancel+delete into one offloaded thread. Signed-off-by: SPEC <zt1y17@soton.ac.uk> * docs(memory): align cancel_by_agent None-scope with legacy root Document that user_id=None cancels only the legacy no-user bucket, matching clear/storage semantics, not the whole process-local queue. Signed-off-by: SPEC <zt1y17@soton.ac.uk> * test(memory): fix cancel_by_agent docstring regression assertion Signed-off-by: SPEC <zt1y17@soton.ac.uk> * fix(memory): address final cancel review nits Type the delete helper with AgentStore, replace docstring pinning with a kwargs mapping test, and document scoped cancel + residual window in AGENTS.md. Signed-off-by: SPEC <zt1y17@soton.ac.uk> * fix(memory): resolve agent store inside delete worker thread get_agent_store() does blocking config/FS work; keep it off the event loop so test_delete_agent_does_not_block_event_loop and backend-blocking-io CI pass. Signed-off-by: SPEC <zt1y17@soton.ac.uk> --------- Signed-off-by: SPEC <zt1y17@soton.ac.uk> |
||
|
qian
|
30788c79ff
|
fix(title): ignore upload context in conversation titles (#4729)
* fix(title): ignore upload context in conversation titles * fix(title): cover attachment-only conversations * fix(title): skip model for attachment-only messages * fix(title): handle whitespace-only user content --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
AoHanBei
|
9b32b5d841
|
feat(observability): persist loop detection events (#5127)
* feat(observability): persist loop detection events * fix(observability): persist subagent loop events * fix(observability): narrow subagent loop event bridge * fix(observability): attribute subagent loop events * fix(tests): isolate subagent executor imports |
||
|
rayhpeng
|
cd35363a05
|
fix(history): early user messages vanish or jump mid-run when pagination and context compaction overlap (#4696)
* fix(history): stop dropping user messages that fall outside the loaded page window Two independent paths made a user's own message disappear from a long thread (#4666, #4508, #4363). Both are reproduced by a real two-round run: once the thread passes the 50-row `/messages/page` window AND context compaction fires, the two sources of truth stop overlapping at the head. 1. Middleware-answered tool results never reached the event store. A middleware that short-circuits a tool call (e.g. ReadBeforeWriteMiddleware's blocked write) returns a user-visible ToolMessage, but LangChain never emits `on_tool_end`, so RunJournal never persisted it — the user saw it during the run and it vanished on reload. RunJournal already reconciles final-output tool messages, but only for an `ask_clarification` allowlist. The allowlist is removed; scope stays bounded by the three conditions that actually matter (visible, this run's lead agent, not already persisted), so subagent results still stay in their own step feed. 2. mergeMessages discarded the checkpoint prefix before the first shared anchor. #4065 correctly established that a summarization-rescued early message must not be appended to the tail, and suppressed it instead. That suppression is what deletes the message when the first history page no longer reaches back to it. It is now woven in before the first shared anchor — the one position both the checkpoint and seq-sorted history agree on — so #4065's invariant (never the tail) still holds. A collapsed unloaded gap is recoverable by paging; a dropped message is not. Verified against real captured payloads from the reproducing run: the first user message returns to the transcript. Its exact position is still approximate — after compaction the live window carries too few anchors to place it precisely, which only seq-based ordering can close. Backend: 10809 passed (baseline 10808; same 15 pre-existing failures in browser/crawler community tools). Frontend: 986 passed, typecheck + eslint clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(events): look up a persisted message's seq by identity Groundwork for placing checkpoint messages in the seq-ordered thread feed (#4666). A checkpoint carries no seq of its own and loses messages to summarization, so once the feed's 50-row page window no longer reaches back to a surviving old message, a client has nothing to place it by. The seq already exists in run_events keyed by the message id — this exposes it without paging the whole feed. `message_identity` is the backend half of the identity rule the frontend applies in `hooks.ts::messageIdentity`: a ToolMessage is keyed by `tool_call_id`, and DynamicContextMiddleware's `X` / `X__user` human copies collapse to one identity. The two halves must stay in sync — a mismatch is silent, degrading placement rather than raising. `get_message_seqs` is implemented for all three stores. Misses are absent from the result rather than an error, so callers degrade to their own placement rule; the earliest seq wins when one identity resolves to several rows, so a re-persisted message keeps the position it first occupied. The DB store decodes rows in Python because `content` is a TEXT column holding a JSON string, not a JSON column — the identity fields cannot be projected in SQL. Nothing consumes this yet; no behavior change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(runtime): carry each persisted message's feed seq on values frames Attaches `additional_kwargs.deerflow_seq` to messages in a root `values` frame that the thread feed already holds, so a client can place a message the checkpoint kept but its loaded history page window no longer reaches (#4666). Nothing is written back to the checkpoint: the seq is added when the frame is serialized and belongs to that frame only. Cost is bounded to frames introducing identities the run has not resolved yet. Messages this run produces are not in the feed while streaming, so they are looked up once, recorded as misses, and never retried — in a real run the only frame that pays for a query is the one where compaction brings older messages back into view. Measured on a reproducing two-round run: 1 lookup across 25 values frames. The stamper is built once per run rather than per `_stream_once`, or a goal continuation would discard the resolved seqs. Subgraph frames are not stamped: a subagent's snapshot is not part of this thread's feed ordering. A lookup failure logs and leaves the frame unstamped rather than failing it — placement is an enhancement and clients fall back to their own ordering rule. Frontend does not read the field yet; no behavior change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(gateway): strip the server-owned message seq from untrusted input `deerflow_seq` is display metadata the Gateway attaches when it serializes a values frame. A client replaying messages (regenerate / edit-and-rerun) would otherwise write it into the checkpoint, where it becomes wrong the moment the thread is forked — a branch re-seeds its feed and reassigns seq (#4380). Joins the existing server-owned key set, so it follows the same trusted-internal rule as the dynamic-context and view-image markers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(frontend): place a checkpoint message by its feed seq, not its nearest anchor Completes #4666. Weaving a compaction-rescued message before the first shared anchor keeps it in the transcript, but not in the right place: after compaction the live window carries too few anchors, and the nearest one can sit deep inside the loaded page window — measured at row 25 of 50 on a reproducing run, which is why the first user turn rendered mid-transcript instead of at the head. Both sides now carry the backend's thread-global seq. `buildVisibleHistoryMessages` copies each row's `seq` onto the message (same shape as the existing `run_id`), and the Gateway stamps it onto `values` frame messages it has already persisted. A live message whose seq is below the loaded window's lower bound is placed ahead of everything on screen rather than before the nearest anchor. A message with no seq — still streaming, so not in the feed yet — keeps the weaving path, since the tail is already its correct position. Verified against the captured payloads of the reproducing run: the first user message goes from absent, to #13 (behind the second question), to #0. Frontend: 988 passed, typecheck + eslint clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(frontend): place a pre-window checkpoint message even when no anchor is shared Also #4666. Placing a compaction-rescued message by its feed seq was gated on reaching a shared anchor, because the split ran inside the anchor walk. When the loaded page and the live checkpoint share no identity at all, that walk never runs and the message fell through to `[...canonical, ...live]` — appended after the entire window, the one arrangement #4065 proved wrong, with its seq known the whole time. That is not a corner case. Open an old, already-summarized conversation and send a message: the page on screen is the newest rows from before that turn, while the checkpoint holds the rescued first user turn plus steps of the new run that are not in the feed yet. On a reproducing run the two sides shared zero anchors and the user's own first question rendered at row 50 of 50 — the reported "first message jumps to the bottom". Split `beforeWindow` out of `live` before walking anchors, walk `liveInWindow`, and use it for the no-anchor branch as well, so a message routed ahead of the window is not re-appended at the tail by dedup. Measured on captured payloads of a reproducing run (real gateway, real compaction), first user message position: no shared anchor: row 50 -> row 0, seq order monotonic again shared anchors: row 0 -> row 0 (unchanged) paged to the top: row 0 -> row 0 (unchanged) Regression test verified red-green: reverting the fix fails it with the message rendered after the window. Frontend: 989 passed, eslint + tsc clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(gateway): stamp the message feed seq on checkpoint reads, not only on stream frames Completes #4666. `_MessageSeqStamper` sits on the streaming publish path, so a client that joins a live run learns where a summarization-rescued turn belongs while a client that merely opens the conversation does not — and opening is the common case. `GET /threads/{id}/state` and `POST /threads/{id}/history` returned the checkpoint with no seq at all, so the merge fell back to the nearest shared anchor, which after summarization sits deep inside the loaded page. Reproduced in a browser against a real gateway, on a thread that had already compacted: the user's first question rendered at row 320 of 389, behind the newest question instead of at the head. Both reads showed 0 of 13 messages carrying a seq. That is the reported symptom, still present after the streaming fix. Add `stamp_messages_with_seq`, the request-scoped counterpart of the stamper: everything a checkpoint still holds is already persisted, so one batched lookup resolves the whole list and there is nothing to retry later. Resolve the store through `_optional_run_event_store` rather than `get_run_event_store`, because seq is placement metadata — a deployment without a feed must still be able to read a thread. After the fix, on the same thread in the same browser: 13 of 13 messages carry a seq and the first question renders at the head, ahead of the newest one. Backend: ruff clean, 326 passed across the touched suites. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(harness): move the injected-user-id suffix helpers to utils.messages to break an import cycle message_identity imported strip_injected_user_message_id_suffix from the dynamic-context middleware, closing a cycle (middleware -> deerflow.runtime -> worker -> events -> middleware) that only stayed hidden while an earlier import happened to break it. Define INJECTED_USER_MESSAGE_ID_SUFFIX and the strip helper in deerflow.utils.messages and re-export them from the middleware so existing importers keep working. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(docs): improve formatting and clarity in AGENTS.md and message-merge.test.ts * perf(events): stop the seq scan once every wanted identity is resolved Rows past the last wanted seq can only be re-persisted copies that already lose the earliest-seq-wins tiebreak, so all three stores now break out of the scan (and the db store out of its per-row JSON decoding) once found covers wanted. Matters most for /state and /history reads of long threads, where this lookup runs with no run cache and a typically tiny wanted set. Raised by review on #4696. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(events): share the seq-stamping expression between the two stampers The walrus-plus-merge expression was duplicated verbatim between stamp_messages_with_seq and _MessageSeqStamper.stamp — two counterparts of one rule where silent divergence is the likely failure mode if only one side is edited. Both now call attach_message_seq next to MESSAGE_SEQ_KEY in message_identity.py. The trailing isinstance(message, Mapping) guard was unreachable (a non-Mapping entry already got identity = None) and is gone with the extraction. Raised by review on #4696. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(events): seq stamping survives launch paths without user context The db store's get_message_seqs defaults to user_id=AUTO, which raises when no user is in the contextvar — the first strict-AUTO read ever called from the worker context. On a launch path that never inherits the auth context (e.g. a null-owner scheduled task), stamp()'s except clause swallowed that into a per-frame warning and silently disabled seq stamping for exactly the background runs that need it. The stamper now soft-resolves the user id once at build time — the same rule as the worker's write paths beside it (unset -> no filter) — and passes it explicitly. jsonl/memory stores gain the same user_id kwarg the base list_messages contract already carries. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * perf(events): SQL-prefilter the message seq lookup's candidate rows get_message_seqs scanned and JSON-decoded every message row of the thread: the early exit never fires when a wanted identity is absent from the feed (a message still streaming, or checkpoint-only), and /state / /history reads want the newest messages, so the ascending scan traversed essentially the whole feed — with the content column carrying full tool outputs, that is heavy I/O plus N JSON parses on exactly the long threads this lookup exists for. A LIKE prefilter now keeps that cost in SQL: only rows containing a wanted raw id as a substring are fetched and decoded. False positives are re-checked by message_identity; LIKE wildcards are escaped; an id json.dumps would escape (breaking the verbatim-substring guarantee) falls the whole set back to the full scan rather than silently missing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(agents): sink runtime mechanism docs below the gateway guidance budget Merging main pushed backend/app/gateway/AGENTS.md past its 40KB soft budget (main had left 81 bytes of headroom). Per the nearest-file rule, move the mechanism detail of the message-seq stamping and run-delivery receipt sections — both owned by runtime/ code — into packages/harness/deerflow/runtime/AGENTS.md, leaving the gateway file the REST-surface summary and a pointer. The seq section also documents the stamper's build-time soft user-id resolution and the db store's SQL prefilter from the review follow-ups. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(agents): sink durable-MCP task detail below the backend guidance budget Merging main pushed backend/AGENTS.md past its 24KB module soft budget (main itself is at 24762 after #4848 — this branch adds zero net bytes to the file). Per the nearest-file rule, move the two durable-MCP task runtime bullets' mechanism detail into packages/harness/deerflow/mcp/AGENTS.md, leaving summaries and pointers; this also restores ~2KB of headroom so the next merge does not trip the same wire. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(events): re-ask a message-seq miss once the feed advances The run-scoped stamper cached lookup misses for the whole run. A message this run produces reaches a values frame before RunJournal flushes it, so its first lookup legitimately misses — and the journal persists it moments later, giving it a feed seq the stamper never asks for again. A long run that afterwards rolls past the history page and compacts then carries that message unstamped, back to the approximate anchor placement this stamper exists to replace (#4666). A transient store error had the same permanent effect, since the except clause degrades to an empty result. A miss is now provisional while a hit stays final: RunJournal counts its successful event-store writes as `feed_generation`, and the stamper re-asks a missed identity only once that counter moves. Retrying is therefore bounded by feed writes rather than by frames — the per-frame query the run-scoped cache was built to avoid — and a failed lookup costs one generation instead of the run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Hyeonsang Cho
|
9146bfa03d
|
feature(gateway): issue request trace ids unconditionally (#5119)
* refactor(gateway): issue request trace ids unconditionally The request trace id was gated behind logging.enhance.enabled at every entry point, so downstream code had to keep asking whether one existed: a header-provenance flag in its own ContextVar, a precedence resolver, and three-level carrier fallbacks at each consumer. Bind one unconditionally instead. TraceMiddleware covers Gateway HTTP; ensure_trace_context covers the entry points that never touch ASGI -- scheduled occurrences, MCP task notification runs, IM channel messages, and the embedded client -- each scoped to one unit of work so a long-lived worker task cannot leak one occurrence's id into the next. The ContextVar becomes the only source; the response header, runtime context, run metadata and log records are derived outputs. Consumers now use ensure_trace_id() or resolve_trace_id(*carriers) and drop their presence guards. Removed: resolve_deerflow_trace_id, the header-provenance flag and its three helpers, set/reset_current_trace_id, is_trace_correlation_enabled and its gateway alias. BREAKING CHANGE: every Gateway HTTP response now carries X-Trace-Id and it cannot be turned off; logging.enhance.enabled controls log output only. Installations on the default enabled: false will start seeing the header. No config keys were added or removed. * fix(gateway): stop persisting a caller-supplied trace id on the run record body.metadata forks two ways: through build_run_config into the live run config, which the run worker restamps, and through create_or_reject into the run record that the runs API echoes verbatim. Only the first was covered, so a client sending metadata.deerflow_trace_id made the most durable and most visible surface of a run disagree with the X-Trace-Id and the log lines the same request produced -- a correlation id that does not match the logs is worse than none. Stamp the server-issued id once at the trust boundary so both forks receive it, preserving the caller's own metadata keys. Close the same gap on config.context, which reaches the runtime context by a separate path: _build_runtime_context no longer merges server-owned keys from the caller, and _install_runtime_context assigns rather than setdefaults. A thread's metadata is no longer seeded with the run-scoped id of whichever run created it -- one thread spans many runs and as many trace ids. Found by driving a real run through the Gateway and reading the run back from the runs API; every unit test built its metadata by hand and so could not see it. * fix(gateway): expose X-Trace-Id to split-origin browser clients X-Trace-Id is not on the CORS safelist, so a browser client served from a separate origin could not read it -- and those are exactly the clients that cannot read the Gateway's logs either, leaving them with nothing to quote in a bug report. Same-origin nginx deployments were unaffected, which is why this stayed hidden. Add it to CORS_EXPOSED_HEADERS beside Content-Location, referencing TRACE_ID_HEADER rather than repeating the literal. * fix(gateway): keep X-Trace-Id on unhandled-exception 500s Starlette's ServerErrorMiddleware sits outside every user middleware and emits unhandled-exception 500s through the raw send, so those responses never pass TraceMiddleware's header-writing wrapper. The 500 for a server bug is exactly the response a user most needs to correlate with a log line, and it was the one response that shipped without the id. TraceMiddleware now tracks whether http.response.start has been sent. On an exception with no response started it emits its own plain 500 carrying the header, then re-raises: the outer ServerErrorMiddleware sees the response already started and only re-raises too, so the server's exception logging is untouched. An exception mid-stream keeps propagating unchanged — a second response start cannot be sent, and the already-written header stands. The trace id is printable ASCII by construction (normalize_trace_id / generate_trace_id), which is what makes the raw latin-1 header encoding safe. * fix(gateway): strip the forged trace id from the persisted request echo The run-record fix stopped a forged metadata.deerflow_trace_id on the authoritative metadata surface, but the raw request echo still carried one: create_or_reject persists body.config verbatim as runs.kwargs_json, which the runs API serves back. A client posting config.context.deerflow_trace_id therefore still got its forged value stored and echoed on one API surface while the header, logs, run metadata, and checkpoint all carried the real id — the id is ignored as input there, so echoing it back only manufactures disagreement. Two changes close it. redact_config_secrets — already the shared scrub for that echo, applied at admission and again at serve time, so historical records are covered too — now also drops deerflow_trace_id from config.metadata and config.context. And build_run_config now merges run metadata onto a copy of the caller's config["metadata"] instead of updating it in place: the nested values of the request config are reference copies, so the in-place merge was writing the server-stamped key through into body.config, contaminating the "what the client sent" record before it was persisted (and incidentally masking the forged-value echo on the metadata container). The regression test posts a forged id through body.metadata, config.metadata, and config.context at once and reads the kwargs echo back off the run record, failing if either leak returns. * docs(harness): record the trace-echo scrub, 500 fallback, and accepted retry divergence The trace section of the harness AGENTS.md now covers the two fixes that close the derived-output rule (the kwargs-echo scrub in redact_config_secrets plus build_run_config's copy merge, and TraceMiddleware's own 500 for unhandled exceptions), and CHANGELOG gains their Fixed entries. It also writes down the one accepted divergence: a crash-recovered scheduled launch reuses the durable run through its idempotency key, and start_run returns early on idempotency_reused without restamping — so the run record keeps the first attempt's deerflow_trace_id while the retry's own log lines carry the freshly minted id of its ensure_trace_context binding. The divergence is confined to the crash-recovery window and is accepted rather than fixed: restamping on reuse would rewrite a persisted record for a run that already exists, which is worse than two ids that each correlate their own attempt's logs. Written down so the next reader of the scheduler recovery path does not diagnose it as a bug. * docs(config): align the logging.enhance schema note with the unconditional trace id The config-module AGENTS.md still described logging.enhance as the gate for the Gateway X-Trace-Id header and Langfuse deerflow_trace_id. That model is gone: ids are issued unconditionally and this block decides log output only. Left as-is, the stale wording invites an agent to "restore" a header gate it believes was lost. Reworded to match the sibling AGENTS.md files and config.example.yaml, with a pointer to the Request Trace Context section that owns the full model. * docs(changelog): link the trace entries to #5119 The five new entries pointed at the ([#XXXX]) placeholder with no reference definition, rendering as literal text instead of a link — and RELEASING.md step 2 relies on those references when the section becomes release notes. All five now point at #5119, with the definition appended to the reference block. * refactor(harness): rename _stream_without_trace_context to _stream_turn The name asserted the opposite of what the method now does. It was accurate while logging.enhance.enabled could route stream() around the trace scope; with the gate gone it is the only stream implementation left, and it binds the id itself via ensure_trace_id(). Private, so the rename touches only the definition and the one stream() call site. * docs(harness): fit the trace-context guidance inside the AGENTS.md chain budget The expanded Request Trace Context section pushed the effective AGENTS.md chain for agents/middlewares to 99,815 bytes, past the 98,304 hard limit scripts/check_agent_guidance.py enforces in CI (AG002). Compressed the section from 7,359 to 4592 bytes with no facts removed: the entry-point table, the derived-output rule and its enforcement points, the accepted scheduled-retry divergence, the two resolution helpers, the stream() binding rationale, the log-output-only gate, the CORS listing, the 500 fallback, and the test map all remain. Sized against the merge, not just the branch: current main grew the same chain by ~724 bytes, so the check was verified on the merged tree as well (97,772 bytes; branch tree 97,048). * fix(gateway): declare content-length on the fallback 500 The pre-response 500 declared content-type but no content-length, leaving the framing to the ASGI server: chunked on HTTP/1.1, close-delimited on HTTP/1.0 — the one wire difference from the ServerErrorMiddleware response it replaces, which sends content-length: 21. The explicit header keeps the fallback byte-identical to what clients saw before. * docs(readme): drop the trace-correlation condition from the translations The zh/ja/fr/ru Langfuse sections still said metadata.deerflow_trace_id matches X-Trace-Id "when request trace correlation is enabled". The id now always matches and that condition no longer exists, so each bullet states the unconditional match and that logging.enhance.enabled only controls whether the id is printed into logs — the one piece of the feature a user can still configure. * test(gateway): pin TraceMiddleware wiring through create_app() Every X-Trace-Id test exercised a hand-built four-route app, so the real stack's add_middleware(TraceMiddleware) line was pinned by nothing: deleting it — or short-circuiting above it — passed CI while silently dropping both the response header and the ambient id the run-record stamp and enhanced log records derive from. One case now drives /health through create_app() and asserts the inbound id round-trips; mutation-checked by removing the wiring line, which fails exactly this test. * docs(gateway): note the fallback 500 is CORS-opaque The pre-response 500 is emitted outside CORSMiddleware — the exception has already unwound past it — so it carries no Access-Control-Allow-Origin and a split-origin browser client cannot read the id on this one response, unchanged from the ServerErrorMiddleware 500 it replaces. Documented on the class and in the CHANGELOG entry rather than fixed: replicating the origin allowlist outside CORSMiddleware would let the two policies drift. * fix(harness): keep abandoned-stream cleanup inside the trace binding stream() binds the turn's id around each next(inner) and resets it before yielding, but the finally's inner.close() ran after that binding was gone. Abandoning the stream therefore drove the inner LangGraph generator's GeneratorExit/finally path with no trace id — or an unrelated ambient one from whichever context ran the close — so cancellation and finalization logs and callbacks did not correlate with the turn they belong to. inner.close() is now wrapped in a local bind/reset of the same turn id. The token is set and reset in the same frame, never across a yield, so the per-step cross-context safety is preserved even when GC closes the generator from another Context — pinned by the existing copy_context close test, which now exercises this path. The regression test records the id from the inner generator's finally and fails without the binding. * test(harness): teach the worker-trace fake about RunManager.cleanup Upstream #5112 (bound gateway memory after terminal runs) added a run_manager.cleanup(run_id) call to run_agent's finalization, so the merge-commit CI run failed all five worker-trace-binding tests with AttributeError on this PR's _FakeRunManager. The fake gains the same no-op shape as its other methods. * docs(gateway): bring the gateway AGENTS.md back under its soft budget Upstream #5092 grew backend/app/gateway/AGENTS.md to 40,966 bytes, 6 over the 40,960 soft budget that test_agent_guidance_check.py::test_repository_guidance_stays_below_soft_budgets_and_avoids_doc_indexes enforces — its Unit Tests run on main was cancelled by push concurrency, so main is currently red on that test and every PR merge-run inherits the failure. Two whitespace/wording trims in the row #5092 touched (a doubled space, and "its configured `context_window`" → "its `context_window`") bring the file to 40,953 with no content change. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Zeren Wang
|
a06a6fed7e
|
feat(harness): deterministic acceptance checklist for subagent delegations (RFC #4651, layer 2) (#5109)
* feat(harness): deterministic acceptance checklist for subagent delegations (RFC #4651, layer 2) PR4 of RFC #4651: check lead-supplied acceptance_criteria in code when a subagent completes, so objectively checkable requirements can never be silently passed by a self-report. - subagents/acceptance_checks.py: deterministic leaf families — file:<path> exists|non-empty and file_written:<path> read through read_current_file_content scoped to the shared thread workspace; the read uses the sandbox-native virtual path form (the local read validator and provider mount tables resolve /mnt/user-data/... paths, not host paths); the scope decision canonicalizes with realpath on the local sandbox so workspace symlinks cannot escape into uploads; a remote provider's "Error: ..." return string is normalized to a failed check (provider-typed via is_local_sandbox); a UnicodeDecodeError marks a binary deliverable as existing and non-empty; out-of-scope paths degrade to UNVERIFIED. tests_passed:<command> anchors to a matching recorded bash execution with status=success and a test-summary shape; matching is shell-structure aware with control-flow attribution (span must end at the last segment with provable execution), negating-option values are ineligible evidence and a target negated anywhere in the command degrades the match, extra flags must be selection-preserving, extra positionals widen only after a path-scoped criterion, truncated commands degrade via command_truncated, the summary shape is read only from output attributable to the matched segment (preceding segments provably silent by invocation form), and pass shapes require a nonzero passed count. Criterion text is neutralized with neutralize_untrusted_tags before storage/rendering. Anything else renders UNVERIFIED, never silently passed. - executor: accumulate bounded bash command/output evidence per streamed chunk (merged by tool_call_id, newest-capped) so subagent summarization compacting earlier messages cannot erase a recorded execution; the recorded status is the actual shell exit status parsed from the output's exit marker (signed codes included; the remote Command exited with code N form is accepted only as the whole trimmed output), falling back to deerflow_tool_meta only when no marker exists. - sandbox providers: e2b/opensandbox/tenki/boxlite append the LocalSandbox-style "Exit Code: N" marker on nonzero exit even with non-empty output; aio propagates the SDK's structured exit_code on both exec paths the same way; local timeouts append Exit Code: 124; and _truncate_bash_output always preserves a trailing exit marker (signed included) inside its budget, with a 32-char floor raising any smaller configured limit, so the actual shell outcome always survives in the output text. - task_tool: run the checklist offloaded (asyncio.to_thread) on the completed branch, failure-isolated; stamp the verdict into result metadata and render the per-criterion section into the model-visible result text. - status contract: additive subagent_acceptance_verdict transport with read-side structural validation. - delegation ledger: entry carries the verdict and renders a compact acceptance segment; gateway strips caller-forged verdicts from both ledger entries and message metadata, like the citation verdict. - blocking-IO anchor pins the offload (teeth proven red->green); leaf read errors catch only OSError/SandboxError so unexpected errors reach the task-tool-level isolation instead of being mislabeled. * fix(harness): close acceptance evidence gaps from review (RFC #4651 PR4) - negating options: overlap with a matched criterion target is now checked by path/nodeid prefix, not exact token equality — excluding a sub-path of the criterion's selection (pytest tests --deselect tests/unit/test_auth.py) degrades to UNVERIFIED instead of holds - output attribution: any redirection token in the matched final segment makes the recorded tail non-attributable (> / >> / 2> are word characters to the parser, so redirection was invisible to the matcher) - silent-source allowlist narrowed from any *activate suffix to the */bin/activate shape - status_contract docstring: restore the shared-fixture sentence and note subagent_acceptance_verdict is deliberately outside the fixture - executor: update_bash_executions publishes [] (stream carried no bash-family calls) instead of collapsing it into None, mirroring update_tool_receipts * fix(harness): close acceptance residual gaps from re-review (RFC #4651 PR4) - tests_passed: add error outcomes to the fail shapes — "4 passed, 1 error" and pytest's "ERROR <nodeid>" short summary no longer satisfy the pass shape when the exit status is swallowed (|| true) or absent; zero-error counts stay clean. - file leaves: bound the deliverable read — a "wc -c" shell size probe answers files above 50k bytes without loading ~2x their size, honoring the host-bash kill switch and falling back to the full read on any non-integer rendering, so verdicts never get less sound. - executor: record the exit marker text as status_marker on harvested bash evidence; the leaf detail now reports the marker actually seen instead of asserting a failure indistinguishable from the command's own trailing text. - extend the blocking-IO anchor to drive the probe branch inside the offload; teeth re-verified red->green. * fix(harness): close acceptance forgery and bound gaps from P2 re-review (RFC #4651 PR4) - file leaves: never read unbounded — size is established first (os.stat on the validated local host path, so the host-bash-disabled configuration needs no shell; a guarded wc -c on remote providers that renders missing/unreadable in its own words). Above the 50k cap the leaf answers from the size alone, at/below it the full read runs, and an unestablishable size degrades to UNVERIFIED instead of an unlimited fallback read. - output attribution: source/. prefixes are never provably silent — a crafted */bin/activate path shape says nothing about what the script prints, so sourced segments can no longer lend a passing summary. - executable identity: an explicitly path-spelled criterion now requires the same normalized executable path; the basename rule stays only for deliberately bare criterion commands. * fix(harness): run acceptance size probe outside subagent-controlled state (RFC #4651 PR4) - remote probe no longer runs in the sandbox's persistent shell: a fresh env -i /bin/sh with absolute-path stat/realpath (poisoned functions, aliases, PATH, exported functions, IFS, locale cannot steer it), plus a marker env routing AIO onto a fresh per-call bash.exec session. - metadata-only: stat never opens content, so a FIFO deliverable cannot block the parent for the provider's idle timeout; non-regular files (fifo/dir/symlink) degrade to UNVERIFIED. - containment canonicalized against the literal mount root: a final-component symlink or a swapped parent directory (root included) cannot redirect the check outside shared storage; unprovable layouts degrade to UNVERIFIED. * fix(harness): canonicalize probe containment against the canonical mount root (RFC #4651 PR4) Literal-root equality made every remote file leaf permanently UNVERIFIED on e2b and Tenki, which realize /mnt/user-data as a symlink to the home dir by default (e2b bootstrap 'sudo ln -sfn', Tenki best-effort symlink). Containment now compares the file's realpath against the mount root's realpath — exactly what the provider's own read path resolves, so probe and read-back stay consistent; final-component symlinks stay rejected by the non-dereferencing stat, and an intermediate dir-link escape under a sane root still lands ESCAPED. The inner script is a module constant and the suite now executes the composed probe for real against on-disk layouts (real dir, symlinked prefix, final symlink, fifo, missing, dir-link escape), which the canned-output stub could not see. * fix(harness): close bare-criterion negation and CDPATH summary channels (RFC #4651 PR4) - matching: a criterion with no positional selection target (bare pytest, make test) stands for the runner's default selection, so ANY negating option (--ignore/--deselect/...) makes the recorded run a different selection — unprovable. The overlap guard only sees consumed criterion tokens, which a bare criterion does not have; scoped criteria keep the unrelated-exclusion behavior. - attribution: cd is no longer blanket-silent — CDPATH makes cd print the resolved (subagent-chosen) destination and the pass shapes match as substrings, so one mkdir 'all tests passed' plus an export minted a pass for any quiet command. A cd argument or CDPATH= value (export or leading assignment) carrying any summary shape makes the segment non-silent; shape-free cd dir wrappers keep matching. - docs: _truncate_bash_output states the effective 32-char floor (the guarantee previously read as an unconditional max_chars bound). * fix(harness): close env-assignment and expansion channels in acceptance matching (RFC #4651 PR4) Self-audit in the shape of the last review rounds — channels the matcher classified as accounted-for that can change what runs, narrow the selection, or lend the summary text: - env assignments are no longer blanket-stripped: only an allowlist of inert display/CI knobs (CI, NO_COLOR, PY_COLORS, ...) may prefix a matched span, and a non-allowlisted assignment in any preceding segment (pure-assignment or export NAME=) is state pollution — PATH redirects the executable, LD_PRELOAD/PYTHONPATH/NODE_OPTIONS inject code, PYTEST_ADDOPTS/GOFLAGS/MAKEFILES inject selection-changing inputs, BASH_ENV runs arbitrary shell startup. All degrade to unprovable. - runtime expansions: any span token carrying /$( )/backticks, any negating-option value carrying an expansion or glob (unknown excluded set), and any extra executed token carrying glob metacharacters (crafted option-looking filenames narrow invisibly) are unprovable. Criterion-side globs stay self-consistent (literal match). - cd: an argument carrying a runtime expansion or glob is non-silent (unknown destination, unknown print); CDPATH= assignments are now handled as state pollution at the match layer, subsuming the value-shape special case. * fix(harness): persistent-shell evidence, exact env sets, option-arity scoping (RFC #4651 PR4) - tests_passed: on a persistent-shell provider (new Sandbox.persistent_shell_sessions capability, set by AioSandbox) every leaf degrades to UNVERIFIED — any earlier call in the shared session could have mutated the state the clean-looking run executed in, and only a fresh controlled session (RFC section 6 verifier) can prove otherwise. The flag is read from the provider registry without acquiring a sandbox. - env assignments: the allowlist is gone — no variable is provably inert across repositories (CI/DEBUG are routinely read by tests). The span's assignment prefix must equal the criterion's exactly (values included, order-insensitive); any assignment or export NAME= in a preceding segment is state pollution. - scoping: positional targets are now read by option arity, so a path embedded in an option (--basetemp=/tmp/p, --junitxml=/tmp/r.xml) never counts as a selection target and an extra positional after such a criterion narrows the default selection it denotes. * fix(harness): stamp shell provenance at harvest, close export/unset and arity gaps (RFC #4651 PR4) * fix(harness): split physical newlines as shell separators in acceptance matching (RFC #4651 PR4) * fix(harness): scope cd wrappers to thread data roots, pin accepted boundaries (RFC #4651 PR4) * fix(harness): preserve criterion connectors, prove file_written readable, fail-closed shell capability (RFC #4651 PR4) * fix(harness): compare only the connector prefix, tolerate trailing criterion semicolons (RFC #4651 PR4) * fix(harness): preserve continuation-line operators, keep ./-spelled executable identity (RFC #4651 PR4) * fix(harness): render criteria single-line so a multiline criterion cannot inject a forged checklist line (RFC #4651 PR4) * fix(harness): reject parent-traversal executable tokens in acceptance matching (RFC #4651 PR4) * fix(harness): reject parent-traversal negated values in acceptance matching (RFC #4651 PR4) |
||
|
betterkite
|
b552b5015c
|
fix(messages): drop legacy <uploaded_files> tag handling (#4826)
* fix(messages): drop legacy <uploaded_files> tag handling (#4212) PR #4174 unified upload-context injection on <current_uploads> (IM and web both flow through UploadsMiddleware), and #4632 documented the current path. This removes the remaining backward-compat parsing of the pre-#4174 <uploaded_files> tag, the final cleanup item tracked by the issue: - deermem: only <current_uploads> is stripped from human turns before memory persistence, and the upload-sentence scrubber drops the legacy tag alternative. - mem0: the mirrored message filter recognises only <current_uploads>. - InputSanitizationMiddleware: remove the legacy tag from the blocked-tag denylist (it existed only because deermem parsed the old tag). - frontend: stripUploadedFilesTag / stripInternalMarkers / parseUploadedFiles and the message-list fallback parse only <current_uploads>; demo thread fixtures are migrated to the current tag. Scope decision: a <uploaded_files> block in pre-#4174 history is now treated as ordinary user content (pinned by tests in both layers) instead of being silently dropped or stripped. * style: apply prettier formatting to stripUploadedFilesTag * fix(uploads): keep legacy <uploaded_files> stripping for display/export only Addresses review feedback on #4826: removing the legacy tag from the frontend display layer made pre-#4174 threads render raw <uploaded_files> XML (with server-side upload paths) in chat, copy data, and JSON exports. The backend cleanup stands — memory pipelines and the sanitization denylist treat only <current_uploads> as an internal marker. The frontend keeps the legacy spelling in its display/export-only utilities (stripUploadedFilesTag / INTERNAL_MARKER_TAGS / parseUploadedFiles and the message-list fallback) so old history renders cleanly without leaking internal paths, while the memory/sanitization scope-decision tests remain unchanged. Frontend tests now pin both spellings: <current_uploads> and legacy <uploaded_files> are stripped from copy data, markdown leak-stripping, and JSON exports. * docs(ui): record accepted display-spoof tradeoff for legacy upload tag Review note (willem-bd): since <uploaded_files> is off the sanitization denylist, a live user can type the legacy spelling and fabricate file chips / hide their own message text in display. Display-only and self-inflicted with no backend semantics, so it is accepted for now; documented at both the message-list fallback and stripUploadedFilesTag. Age-gating the legacy spelling remains a possible follow-up. --------- Co-authored-by: betterkite <313258397+betterkite@users.noreply.github.com> |
||
|
Yufeng He
|
a4f6665ef4
|
fix(security): sanitize MCP-sourced tool results through the same trust boundary (#4839)
* fix(security): sanitize MCP-sourced tool results through the same trust boundary Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com> * fix(security): sync the trust-boundary docs with tag coverage and pin the untagged branch Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com> --------- Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com> |
||
|
Aari
|
317577e285
|
fix: enforce custom agent skill allowlists in sandboxes (#5077)
* fix: enforce agent skill allowlists in sandboxes * fix: guard E2B skill projection resets * fix: preserve agent skill isolation across delegation * fix: close sandbox skill isolation bypasses * fix(sandbox): close skill isolation review gaps * fix(sandbox): harden skill isolation lifecycle |
||
|
Yuzhong Zhang
|
8eda71fd97
|
fix(agents): normalize Command-wrapped tool results (#4977)
* fix(agents): normalize Command-wrapped tool results Command-wrapped ToolMessages skipped result metadata and progress tracking, so error receipts could be recorded as success. * fix(agents): stamp error meta from subagent_status failures Delegated task Commands leave ToolMessage.status at success and do not use an Error: content prefix, so normalize_tool_message was labeling failed/cancelled/timed_out results as success. Honor structured subagent_status before content heuristics and cover the four statuses. * style: ruff-format tool_result_meta tests --------- Co-authored-by: Yuzhong Zhang <BetterAndBetterII@users.noreply.github.com> |
||
|
PeaceMaker-best
|
137a3cb60d
|
fix(authz): recheck policy before sandbox reuse (#5006)
* fix(authz): recheck policy before sandbox reuse * fix(authz): avoid duplicate async sandbox checks * fix(authz): scope sandbox decision across middleware * fix(authz): construct async providers on the event loop * test(authz): avoid cold imports under Blockbuster --------- Co-authored-by: 嗜鵼 <hy2010hy2010@qq.com> Co-authored-by: PeaceMaker-best <221849497+PeaceMaker-best@users.noreply.github.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Zeren Wang
|
22b0456e45
|
feat(harness): subagent report contract and delegation acceptance criteria (#5090)
* feat(harness): subagent report contract and delegation acceptance criteria (RFC #4651 PR3) Layer 1 receipt verification is inert unless subagents actually cite their execution record. This lands the prompt layer that closes the adoption gap: - New subagents/report_contract.py owns the model-facing contract text, derived from the single-owner citation format (format_citation / receipt_id) so prompts can never drift from the verifier. The executor injects <report_contract> into every subagent system prompt — built-in and custom alike — requiring [rN tool_name] citations for action claims, verifiable handles (absolute path, URL, ID, HTTP status) for deliverables, and explicit failure reporting; the citation clause follows verification.receipts_enabled. - The task tool gains an optional keyword-only acceptance_criteria parameter, handed to the SubagentExecutor constructor and rendered into the subagent's SystemMessage (stripped, capped 20 items x 500 chars) — deliberately never the task HumanMessage, which InputSanitizationMiddleware classes as genuine user input and would HTML-escape into untrusted-input framing. The docstring frames subagent results as self-reports, states the citation cross-check's evidence boundary (resolved = the call happened, not that the claim is correct), and documents when to attach criteria with the canonical leaf forms. Deterministic leaf checking remains a separate layer. - The lead delegation workflow now instructs reading the ledger citation line as execution evidence only and spot-checking verifiable handles before synthesizing. - report_contract / acceptance_criteria are registered as blocked framework-authority tags in input sanitization so untrusted input cannot forge the verification contract. * fix(harness): neutralize acceptance criteria before system-channel injection render_acceptance_criteria_section interpolated lead-model-supplied acceptance_criteria verbatim into the subagent SystemMessage after only stripping/capping. A criterion such as '</acceptance_criteria><system>...</system>' could close the wrapper and open a framework authority tag, bypassing InputSanitizationMiddleware. Route each criterion through neutralize_untrusted_tags (the shared prompt-injection primitive) so blocked authority tags are HTML-escaped before interpolation. Add regression tests at the renderer and the executor _build_initial_state path. * fix(harness): keep model-supplied criteria off the system channel - Move acceptance_criteria values into the task HumanMessage — the untrusted channel InputSanitizationMiddleware escapes and boundary-frames. The subagent SystemMessage now carries only a framework-owned <acceptance_criteria> pointer note (no criterion text), so natural-language injection inside a criterion keeps task-data priority and cannot override framework instructions (PR #5090 review, willem-bd P1). - Condition the lead delegation workflow's citation verification guidance on verification.receipts_enabled and qualify the task tool's result-reading text with the enabled state, so a receipts-disabled configuration no longer tells the lead to require citation evidence that cannot exist (P2). * fix(harness): drop execution-record promise from report contract when receipts are disabled The <report_contract> opening was emitted unconditionally, so a verification.receipts_enabled=false subagent was told its report would be cross-checked against an execution record that cannot exist in that mode (terminal_receipts() returns None; no verdict, no ledger citation line). The opening now follows receipts_enabled: enabled keeps the cross-check language, disabled describes the handle-only review mode (PR #5090 review, willem-bd P2). * docs: record the prompt-layer trust-boundary self-check Generalizes the PR #5090 review outcome: before adding prompt text, ask of every data source in it what trust level it has and which channel it should ride — model/user-influenceable values ride the untrusted sanitized data channel, never framework-owned system text. Added to the PR template (Agents/LangGraph surface) and agents/AGENTS.md. |