mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-09-22 12:36:26 +00:00
27 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Hyeonsang Cho
|
9f79ddf9b6
|
fix(nginx): allow model-bound /api/ and /api/skills requests past 60 seconds (#5524)
* fix(nginx): allow model-bound /api/ and /api/skills requests past 60 seconds Two locations were left on nginx's 60s default while the routes behind them wait on Gateway. The /api/ catch-all carries the stateless POST /api/runs/wait, which blocks on wait_for_run_completion and cancels its run when the client disconnects, so a caller waiting on a longer run got a 504 and lost the run; it also carries POST /api/input-polish, which waits for a one-shot model call. /api/skills carries POST /api/skills/install, which runs one LLM security scan per file in the archive, and the custom-skill edit and rollback routes, which run one more each. None of them sets an application-level timeout, and only the sibling /api/skills/install/upload endpoint had been given the longer timeout, so the same archive failed at 60s depending on which endpoint installed it. Allow 600s on both locations, matching /api/langgraph/ and /api/threads, in all three copies of the nginx config. Each directive is pinned by its own test that parses the active directive per config. * docs(changelog): link the nginx /api/ and /api/skills timeout entry to #5524 |
||
|
Hyeonsang Cho
|
3cfc9c58fd
|
fix(nginx): allow model-bound /api/threads requests past 60 seconds (#5505)
* fix(nginx): allow model-bound /api/threads requests past 60 seconds The browser calls /api/threads/* directly rather than through /api/langgraph/, and the generic `location ~ ^/api/threads` block set no proxy_read_timeout, so nginx's 60s default applied. /compact and /suggestions hold the response open for a whole model call, and /runs/wait for a whole run. Past 60s nginx returned 504 mid-work: the compaction still committed behind the failed request, and /runs/wait cancelled its run on the disconnect (on_disconnect defaults to cancel). Allow 600s on that location, matching /api/langgraph/, in all three copies of the nginx config: Docker, local dev, and the Helm ConfigMap. The regression test parses the active directive per config, so a missing, commented-out, lowered, or misplaced timeout fails. * docs(changelog): link the nginx /api/threads timeout entry to #5505 --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Willem Jiang
|
849d4910ad
|
docs(changelog): complete the 2.1.0 milestone entries in both languages (#5520)
Every merged pull request in milestone 2 (765, confirmed by paginating the GitHub GraphQL milestone query) now has an entry in CHANGELOG.md, and the Chinese mirror carries the same 119 new blocks with its reference block rebuilt to match. The cited sets are identical between the two files (962 each, the extra 197 being pre-2.0.0 history), with no orphan and no unused reference in either direction. Section skeletons match too: 42 headings in the same order with the same nesting. Two merge defects were fixed while splicing the Chinese entries, both of which would have corrupted the file silently. Anchors were searched across the whole English block list, so a walk could cross a `### ` boundary and land on a bullet from the previous section -- four `新增/调度器` entries were about to be filed under Breaking Changes; anchors are now confined to each entry's own section and subsection, with an assertion at merge time. And the one entry whose Chinese counterpart already narrated the same pull request without citing it had its replacement inserted at the first line the merge then deleted, so the deletion pass discarded the new text and kept the old block; the splice is now a single forward pass over original line indices. Verification lives in /tmp/zhwork (temporary): translation validation, the splice, and a final pass asserting milestone coverage, reference hygiene, cited-set parity, section agreement for all 119 blocks, and line widths. Co-authored-by: Claude <noreply@anthropic.com> |
||
|
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> |
||
|
Hyeonsang Cho
|
53798b44cd
|
fix(subagents): scale max_turns into the graph's super-step budget (#5485)
* fix(subagents): scale max_turns into the graph's super-step budget
max_turns was handed to LangGraph as recursion_limit, but the two count
different things. recursion_limit counts super-steps, one per graph node,
and create_agent compiles a node for every middleware lifecycle hook, so
one turn costs before_model + model + after_model + tools nodes — seven to
eight through the subagent chain. The built-in general-purpose agent's
max_turns=150 therefore bought about 18 tool-using turns before failing as
turn_capped, and every middleware added to the chain shrank the effective
budget again.
Resolve the limit from the chain each subagent was actually assembled with
(subagents/turn_budget.py) instead of passing the turn count through, so
raising max_turns buys the turns it names.
No config keys or defaults changed; existing max_turns values now grant
their full budget, bounded as before by subagents.timeout_seconds and
subagents.token_budget.
* fix(subagents): warn when a counted hook can jump the agent loop
Review follow-up. The per-turn cost is a flat multiplier over the straight
before_model -> model -> tools loop. A hook that declares can_jump_to and
returns {"jump_to": ...} re-enters the loop without traversing tools,
spending another before_model + model + after_model pass that buys no tool
result, so the resolved limit becomes a lower bound rather than an exact
budget — silently re-creating the short budget this translation fixes.
Measured against a compiled graph: with one jumping after_model hook,
three tool turns need the resolved limit plus one jump pass, and the run
raises GraphRecursionError at the resolved limit.
How often a jump fires is data-dependent and unbounded, so it cannot be
folded into the arithmetic. find_jumping_hooks reports the condition off
the same __can_jump_to__ attribute the factory reads, and the executor
warns when a counted hook declares one. Nothing in today's subagent chain
does, so this changes no budget.
* fix(subagents): detect jumps declared on agent-level hooks too
Review follow-up. find_jumping_hooks exempted before_agent/after_agent on
the grounds that a jump out of them lands in the loop the budget already
pays for. That does not hold on langchain 1.3.14:
- after_agent jumps re-enter the loop after it finished, and the hook runs
again on the next exit, so the extra passes are unbounded. Even
jump_to "end" is routed to exit_node, the head of the after_agent chain,
so it reruns the chain; destinations are no safe filter.
- a before_agent hook that stages a tool call and jumps to tools runs a
tools step no model turn paid for. It is O(1), but the resolved limit
has zero headroom, so one step caps the last turn.
The detector now scans every hook pair the factory wires jump edges for.
The compiled-graph pin is parametrized over after_model->model,
after_agent->model, after_agent->end and before_agent->tools, each raising
GraphRecursionError at the resolved limit and completing once the jump's
cost is added. No middleware in the subagent chain declares a jump on any
hook, so this changes no budget.
|
||
|
Hyeonsang Cho
|
b0cb3a3a8c
|
fix(scheduler): serialize the SQLite launch-budget claim (#5469)
* fix(scheduler): serialize the SQLite launch-budget claim claim_queued_run counts the executing occurrences and then promotes one row to launching. Postgres serializes that pair with a transaction advisory lock; SQLite had no counterpart. pysqlite does not begin a transaction for a SELECT, so the budget count ran in autocommit and the deferred transaction reserved the writer only at the promoting UPDATE. Claimers racing on distinct rows therefore read the same stale count, each passed its own status == 'queued' CAS, and max_concurrent_runs was exceeded. A manual trigger overlapping the poller reaches this concurrently within one process, and scheduler.multi_instance over a shared database file reaches it across processes. Two claims of the same row were already safe, which is why the existing coverage did not catch it. Take the writer before the count with BEGIN IMMEDIATE, the idiom ThreadMetaRepository already uses for its read-modify-write paths and the same reservation _lock_task makes for a parent row. The claim targets one row but the budget is global, so this has to be the database-wide writer rather than a row lock. * docs(changelog): reference #5469 in the SQLite launch-budget entry * test(scheduler): pin the launch-budget test's connection reuse The warm-up gather is what makes the claimers actually overlap, but it silently depended on the SQLite engine keeping pooled connections. If that engine ever moved to a non-pooling class, every claimer would open its own connection, the per-connection PRAGMA setup would stagger them, and this test would pass against an unserialized claim instead of failing -- the cold-pool case it exists to avoid. Assert that the warm-up left connections checked in. A non-pooling class does not implement checkedin() at all, so a missing counter reads as zero reuse and reports the same explanation rather than an AttributeError. Verified against NullPool: the guard fails with "NullPool left 0 connections pooled after the warm-up". Only pool_size connections survive the gather (the overflow is discarded), which is why the pre-fix failure is exactly five claimants over a cap of one rather than eight. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Huang-404-Q
|
6bab87aca4
|
fix(sandbox): report an exactly-full AIO glob result as complete (#5449)
* fix(sandbox): report an exactly-full AIO glob result as complete AioSandbox.glob's include_dirs branch returned as soon as it had collected max_results matches, without looking at the rest of the listing. A listing that held exactly that many matches and nothing more was therefore reported as truncated, and the glob tool told the model the result was incomplete — prompting a re-search or distrust of a complete answer. The same line returned one match for max_results=0, one past the caller's cap. Look one match past the cap before deciding, which is what the include_dirs=False branch in the same function already does and what #5427 moved parse_remote_search_output to for BoxLite, Tenki, E2B and OpenSandbox. * review: filtered-tail cases, the glob contract docstring, and the cap wording Addresses the three items from the review on #5449. - Two regression cases over a tail of ignored / out-of-root / pattern-miss entries: an exactly-full result stays complete when only filtered entries follow, and a third eligible match after that tail still reports truncation. Both fail against the previous return-on-the-max-th-match behaviour. - 'Sandbox.glob' promised the conservative flag ('``max_results`` was reached') that this change deliberately stops producing on the AIO branch. The contract now reads as 'may be incomplete' and records that providers differ in how precisely they can decide it. - The changelog no longer lumps 'parse_remote_search_output' in with the filtered-match cap: its raw-output cap is a separate limit with its own one-line-past accounting, and the other providers' filtered-match cap is unchanged. Also corrects the docstring on the existing test, which still described the removed early return in the present tense. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.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. |
||
|
Hyeonsang Cho
|
6469833886
|
fix(skills): close SkillScan bypasses in the skill review gate (#5431)
* fix(skills): close SkillScan bypasses in the skill review gate The public skill review gate re-materialized a package snapshot into a temp directory for SkillScan, but copied only entries the reader had decoded as text and skipped every file under any evals/fixtures/ directory. Executable binaries and nested archives never reached the package rules, and a fixture-shaped path hid any script from the scan. Readers now keep binary bytes as content_base64, the analyzer writes every non-symlink file byte for byte, and only eval fixture SKILL.md samples stay exempt. Files are created exclusively, so a duplicate archive member or a case-folded name fails the scan closed instead of overwriting an earlier file. SkillScan itself skipped any file that was not NUL-free UTF-8. One Latin-1 byte in a comment hid a reverse shell from the review gate, and a NUL byte skipped static analysis at install. Code files that fail strict decoding now raise package-undecodable-script (HIGH) and are analyzed over a lossy decode, so CRITICAL matches keep blocking. "Code file" and "executable magic" were defined separately in the installer and SkillScan and had drifted: SkillScan missed 32-bit little-endian and fat Mach-O variants the installer blocks. Both rules now live in skills/package_files.py, shared by the installer, the export guard, and SkillScan. * docs(changelog): link the skill review gate fix to #5431 * fix(skills): fail closed on bytes-less snapshot entries and skip text rules for executables The review analyzer skipped any snapshot entry it could not turn into bytes. Readers only emit such entries for oversized files, and they also mark the snapshot truncated, but content_base64 is optional in the contract, so a reader regression or a hand-built snapshot would silently drop a file from SkillScan. An entry without bytes now fails the scan closed (not_assessed: skillscan) unless the snapshot is truncated, and a text entry without content no longer materializes as an empty file. A real executable under scripts/ is a code file, so SkillScan decoded it lossily and ran the text rules over its string tables. An OpenSSH binary produced a CRITICAL secret-private-key finding from the key-format banner it embeds. An undecodable file with executable magic still reports package-undecodable-script, and its CRITICAL package-executable-binary finding already blocks it, so it now skips the text rules. Decodable files keep full text analysis. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Hyeonsang Cho
|
ed986a10ef
|
fix(sandbox): report truncated remote glob and grep results (#5427)
* fix(sandbox): report truncated remote glob and grep results
BoxLite, Tenki, E2B, and OpenSandbox run find/grep in the sandbox, cap
the raw output with `| head`, and then filter those lines in Python:
ignored directories such as node_modules are dropped and grep's glob
scope is applied. They reported truncated only when max_results matches
survived the filter. When the capped lines were mostly filtered out, a
search with real matches past the cap came back short or empty with
truncated=False, and glob_tool/grep_tool rendered it as "No files
matched" / "No matches found". With the default max_results=200 and
1,200 files under node_modules, glob("**/*.py") reported no matches for
a workspace that has src/app.py.
remote_search_command now lets one line past its limit through, and
parse_remote_search_output(..., limit=) returns RemoteSearchOutput(text,
truncated): the first `limit` lines and whether the extra line arrived.
Exactly `limit` lines stays a complete result. Each provider passes the
cap it already computed to both calls and returns that truncated from
glob and grep when fewer than max_results results survive filtering.
The glob and grep tools now describe an empty truncated result as
incomplete instead of reporting no matches, which also covers AIO grep's
forwarded truncated flag. Sandbox.glob/grep document truncated as "the
matches may be incomplete".
* docs(changelog): reference #5427 in the remote search truncation entry
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
|
||
|
Hyeonsang Cho
|
1b9667ea0e
|
fix(sandbox): mask every host path in a colon-joined list (#5418)
* fix(sandbox): mask every host path in a colon-joined list Host-to-virtual output masking matched a host root and then consumed the path tail up to whitespace or shell punctuation, but not `:`. A `:`-joined list such as $PATH or $PYTHONPATH was therefore swallowed into the first match's tail, and scanning resumed after it, so every later entry under the same root reached the model as a raw host path. The regex matcher (process-stable skill roots) and the direct scanner (per-thread roots, LocalSandbox) shared the gap. Each redundant masking pass -- separator variants, the realpath spelling, the /mnt/user-data root mapping, LocalSandbox's own reverse resolution -- happened to recover one entry, which hid the leak for short lists: bash output leaked from the fourth entry, single-pass consumers from the third. The shared tail in path_patterns.py now ends at `:` in both matchers. `;`, the Windows list separator, already ended it. A `:` inside one path (grep -n output, a file name) only shortens the match; the remaining text is copied through verbatim. Shortening the match exposed a second leak. LocalSandbox reverse resolution realpaths the matched path and returned that realpath when no mount contained it, so a symlink inside a mount whose target lies outside every mount was shown as the target's host path. grep -n lines used to hide this only because the whole line resolved as one nonexistent file; whitespace-terminated output and LocalSandbox.glob results already leaked it on main. Reverse resolution now falls back to the link's own spelling, normalized so `mount/../x` does not pass, before giving up. A symlink into another mount still reports that mount's path. * docs(changelog): reference #5418 in the colon-joined path masking entry * docs(changelog): split the #5418 and #5419 entries fused by the merge Resolving the CHANGELOG conflict when main was merged in dropped the opener of the #5419 entry, so the BoxLite grep fix continued inside this PR's bullet in both CHANGELOG.md and CHANGELOG_zh.md. Restore it as its own bullet; the #5419 entry is byte-identical to main again. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Hyeonsang Cho
|
18b803fad9
|
fix(sandbox): scope BoxLite grep globs to the search root (#5419)
* fix(sandbox): scope BoxLite grep globs to the search root BoxliteBox.grep omits grep --include for busybox portability and applies the glob in Python, but it kept only the glob's last path segment and matched it against each file's basename. A scoped pattern therefore lost its directory part: grep(glob="src/*.js") returned every .js file in the tree, including vendor/ and nested src/ subdirectories that glob() with the same pattern excludes. The glob now goes through path_matches against the path relative to the search root, with the file's basename when the root is a single file -- the same scope glob() uses and the one Tenki, E2B, OpenSandbox, AIO and LocalSandbox already enforce. Like those providers, an empty glob is now passed to path_matches instead of being treated as no filter. * docs(changelog): reference #5419 in the BoxLite grep glob scope entry |
||
|
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>
|
||
|
Hyeonsang Cho
|
1dd48d14d2
|
fix(models): reuse the Claude Code OAuth token read from a file descriptor (#5411)
* fix(models): reuse the Claude Code OAuth token read from a file descriptor ClaudeChatModel accepts a Claude Code OAuth token through CLAUDE_CODE_OAUTH_TOKEN_FILE_DESCRIPTOR, but that handoff can be drained only once: a pipe returns EOF after the first read and a file descriptor keeps its advanced offset. _read_secret_from_file_descriptor read it again on every call and kept nothing. Every ClaudeChatModel instance loads credentials in model_post_init, and create_chat_model builds fresh instances per run, so with a descriptor-only handoff the first model authenticated and every model after it -- including the title model of the very first run -- had no credential. The Anthropic SDK then raised "Could not resolve authentication method" before sending a request. A secret read from a descriptor is now kept for the life of the process, keyed by (env_var, fd), so a different descriptor is still read fresh. The read happens under a lock so two threads building their first model concurrently cannot race one of them to EOF. Empty reads and OSError are not cached and behave as before; lookup order, config keys, and log messages are unchanged. * docs(changelog): reference #5411 in the Claude Code OAuth descriptor fix entry * test(models): pin that a closed descriptor handoff keeps its token Review follow-up on #5411: the descriptor secret cache is keyed on the fd number, which the OS recycles. Folding os.fstat identity into the key would break the property the cache exists for -- once the handoff fd is closed after the first read, fstat raises EBADF and every later model would lose the token again -- and it would still miss a regular file rewritten in place, which keeps its st_dev/st_ino. The handoff is fixed at process start, so keep the number as the key and state the invariant instead: a closed handoff keeps serving its token, a secret placed on a recycled number is not re-read, and anything handing over a new secret in-process must clear the cache. A new test pins the closed-handoff behavior; an fstat-fingerprinted key fails exactly that test. |
||
|
Hyeonsang Cho
|
2814bd5d49
|
fix(models): stop reasoning_effort from reaching the constructor twice (#5403)
* fix(models): stop reasoning_effort from reaching the constructor twice The regular lead-agent build forwards reasoning_effort to create_chat_model even when neither the request nor the custom agent chose one. The factory spread that kwarg next to the profile settings, so any profile that also yielded reasoning_effort -- a top-level value, when_thinking_enabled, when_thinking_disabled, or the minimal effort the extra_body.thinking disable path injects -- made the constructor raise "got multiple values for keyword argument 'reasoning_effort'", and the lead agent could not be built for that model. #2017 moved the factory-injected value out of kwargs but left the caller-supplied one. The requested effort now leaves kwargs once and layers like model_overrides: a non-None value replaces the profile value, None keeps it, and the thinking transforms applied afterwards still decide the final value. Codex keeps resolving the requested value itself, so a level it does not accept and a profile without effort support still fall back to medium. * docs(changelog): reference #5403 in the reasoning_effort collision fix entry |
||
|
Hyeonsang Cho
|
6f81daefff
|
fix(runtime): resolve omitted run owners before idempotent reuse (#5401)
* fix(runtime): resolve omitted run owners before idempotent reuse HTTP run admissions do not pass a user_id. The SQL run store stamps the request user from the contextvar onto the row, but RunManager kept None on its process-local RunRecord, so the two disagreed about who owns the run. A keyed retry that reached a peer worker, or the owning worker after cleanup() released its local record, hydrated the stamped row, compared its owner with None, and raised "Run idempotency key resolved to a different thread or user", which start_run surfaced as a 500. MemoryRunStore stored None on both sides and never hit the check. The same mismatch hid HTTP runs from owner-scoped history reads, which filter local records by the current user, and skipped the worker's MCP background_tasks projection, which only runs for records with an owner. create() and _admit_thread_operation() now resolve an omitted owner from the current user before building the record, and keep None when no user is in context instead of falling back to the default bucket, so the local record and every store agree on the owner. HTTP runs now receive the background_tasks projection, so the replay golden's values frames gain that key. * docs(changelog): reference #5401 in the keyed retry owner fix entry * test(runtime): close the SQL engine when peer-reuse test setup fails Review follow-up on #5401: the sql case of the two-worker start_run test initialized the engine above the try whose finally calls close_engine(). init_engine() assigns the module-global engine and session factory before bootstrapping the schema, so a failure there skipped the teardown and left a stale engine for later tests in the same process. Store setup and the RunManager workers now live inside the try, so the teardown runs whether setup or the test body fails. |
||
|
Hyeonsang Cho
|
28a81452ce
|
fix(runtime): stop idempotent reuse from blocking the thread on a peer worker (#5393)
* fix(runtime): stop idempotent reuse from blocking the thread on a peer worker When an idempotent run admission lands on a worker that does not own the run, RunManager hydrates the stored row and returns it as the reused record. It also registered that row in the worker's local run map, but only the owning worker's task lifecycle finalizes and cleans up local records, so the copy kept its admission-time pending/running status for the life of the process. On that worker every later reject-strategy admission for the thread returned 409 until a restart, run reads kept reporting the stale status, orphan reconciliation skipped the run as locally live if the owner crashed, and a cancel took the local-owner path and marked the owner's still-running row interrupted. Return the hydrated row as a detached store-only handle instead of registering it. A local record for the key is already returned before the store insert, so the removed lookup of an existing local record was unreachable. get(), cancel() and reconciliation now read the durable row on the peer, matching the documented non-owner contract. * test(runtime): pin keyed retries of a terminal reused run on the SQL store Review follow-up on #5393: the post-cleanup release relied on a keyed retry resolving through the terminal row's idempotency conflict, but only MemoryRunStore pinned that path, and no test retried on the owner after its local record was cleaned up. The SQL repository test now retries the key on the peer and on the owner once the run is terminal and cleaned up, asserting both get the same run back as a store-only reused handle before the keyless follow-up is admitted. |
||
|
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> |
||
|
Hyeonsang Cho
|
806a5bd427
|
fix(gateway): serve XML artifacts as attachments to block same-origin script (#5353)
* fix(gateway): serve XML artifacts as attachments to block same-origin script
GET /api/threads/{id}/artifacts/{path} forced only text/html,
application/xhtml+xml and image/svg+xml to download. Every other XML
document was served inline from the application origin: `.xml` guesses
to text/xml or application/xml depending on the host's mime.types, and
both fell through to the inline text branches. Browsers render any XML
MIME type as a document and run an XHTML-namespaced <script> inside it,
so a report.xml written by a prompt-injected agent and opened from a
chat link executed with the viewer's session: the HttpOnly access_token
rides same-origin fetches, and the double-submit csrf_token cookie is
JS-readable, so state-changing calls are reachable as well.
Treat HTML plus every WHATWG XML MIME type (text/xml, application/xml,
any +xml subtype) and text/xsl, which Blink also renders as XML, as
active content. A single helper owns the rule for both the regular-file
and the .skill-archive-member branches. The artifacts panel already
previews .xml as code through a ranged fetch, so preview and editing
keep working against the attachment response.
* docs(frontend): name XML among the artifacts the Gateway downloads
Review follow-up on #5353: resolveArtifactOpenURL's comment still named
only HTML/SVG as the active content the Gateway serves as a download.
XML documents now join that bucket, so the frontend note matches the
Gateway rule. Comment-only; no behavior change.
|
||
|
Hyeonsang Cho
|
556975f284
|
fix(gateway): gate github_token and disable_clarification on internal callers (#5338)
* fix(gateway): gate github_token and disable_clarification on internal callers `non_interactive` is honored only for internally-authenticated callers because it strips `ask_clarification` from the lead-agent toolset. The two sibling run-context keys reproduced that effect without the gate. `merge_run_context_overrides` forwarded `_CONTEXT_RUNTIME_ONLY_KEYS` regardless of `internal`, and `strip_internal_context_keys` scrubbed only `_CONTEXT_INTERNAL_CALLER_KEYS` -- so any session or PAT caller could set `disable_clarification` through `body.context`, or through the free-form `body.config` that `build_run_config` copies verbatim. That is not a milder flag than `non_interactive`: ClarificationMiddleware answers every clarification -- `risk_confirmation` included -- with "proceed without asking" instead of interrupting, and SandboxMiddleware reads the two keys as the same non-interactive signal. `github_token` rode the same path into `runtime.context`, where the bash tool exports it as `GH_TOKEN`/`GITHUB_TOKEN`, and a copy smuggled through `body.config['configurable']` reached the checkpoint store the context-only rule exists to avoid. Both keys are produced server-side by the channel run policies, which reach the Gateway over the internally-authenticated request channel, so gate them the same way: forward them only when `internal=True`, and scrub the union `_INTERNAL_ONLY_CONTEXT_KEYS` from both config sections for every other caller. Destination stays an orthogonal axis -- `_CONTEXT_RUNTIME_ONLY_KEYS` still land in `context` alone, never in checkpoint-persisted `configurable`. Regression coverage in tests/test_gateway_services.py pins both smuggling surfaces and replays the real start_run assembly order for a session caller and for an internal one, so the GitHub channel keeps carrying its minted token. * docs(changelog): record the internal-only run-context key gate (#5338) * docs(agents): keep the run-context note inside the AGENTS.md budgets The AG002 inherited-chain check failed at this head. The new backend section and the root scheduled-task sentence added 993 B to the root and backend guidance both the sandbox and middlewares chains inherit, pushing sandbox 6 B over the 98304 B hard limit and growing the middlewares chain, which main already exceeds by 155 B. An already-over chain is only tolerated while it does not grow, so the shared ancestors had to come back to their base size. Condensed the new material and removed prose the root file was duplicating: - The trust-boundary section keeps both gated surfaces, both helpers, the trust-vs-destination split, and the disable_clarification note in half the space. - The root scheduled-task bullet names all three internal-only keys and both smuggling surfaces while staying under its previous size. - Dropped the root `scheduler.recursion_limit` bullet, which restated backend/AGENTS.md:18 almost verbatim; its one unique fact (a YAML edit needs no Gateway restart) moved to that bullet. - Deduplicated the nginx routing sentence, which already deferred to the backend routing table, and tightened the waiver note's sequencing tail. Root and backend guidance now sit 50 B under their combined base size, so the sandbox chain returns to 97310 B and the middlewares chain no longer grows. Every file stays under its AG001 soft budget. |
||
|
wutongyuonce
|
48a8978b7b
|
feat(scheduler): add interval schedule type (#5291)
* feat(scheduler): add interval schedule type Allow scheduled tasks to fire every N seconds from last dispatch, not only wall-clock cron or a single run_at. Cadence is UTC now+N with no missed-beat catch-up, bounded by min_once_delay_seconds and 30 days. * fix(scheduler): let interval tasks create, edit, and keep next run Create/edit now keep every_seconds. Unchanged interval spec no longer resets next_run_at, including timezone-only PATCH. * fix(scheduler): keep non-minute intervals on edit Stop rounding every_seconds to whole minutes in the form. Values that are not whole minutes or hours now use a seconds unit so edit/duplicate round-trips the stored cadence instead of rewriting it and resetting next_run_at. Document that min_once_delay_seconds is also the interval floor. * fix(scheduler): clamp interval seconds to the default 60s floor The new seconds unit allowed 1–59, which the API rejects under the default min_once_delay_seconds. Clamp the form to >= 60 and show the floor next to the preview. Also mention interval in the scheduler field_doc, matching config.example.yaml. * fix(scheduler): do not clamp interval amount while typing Keystroke clamp made 90 become 9 -> 60, then 600, and backspace could not leave 60. Keep the raw field text and apply the 60s floor on blur and emit only. * test(scheduler): cover interval input editing * fix(frontend): preserve saved interval cadence until edited * style(tests): format scheduled task router tests --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Hyeonsang Cho
|
8e86729aa0
|
fix(gateway): confine artifact PUT to /mnt/user-data/outputs after path resolution (#5321)
* fix(gateway): confine artifact PUT to /mnt/user-data/outputs after path resolution
The outputs-only guard on PUT /api/threads/{id}/artifacts/{path} was a
string-prefix check on the raw path. A percent-encoded `..`
(`outputs/%2e%2e/uploads/x.txt`) survives nginx's variable proxy_pass
untouched, is decoded by Starlette, passes the prefix check, and the
resolver only confines the result to `user-data/` -- so an owner could
overwrite a sibling upload or workspace file in their own thread.
Collapse dot segments before the prefix check, and re-check the resolved
host path against the resolved outputs root so a symlink planted inside
`outputs/` cannot redirect the write either. The normalized virtual path
is what the response echoes and what non-mounted sandboxes receive.
* refactor(gateway): share the outputs-confinement rule with channel attachments
Review follow-up on #5321: the "only under /mnt/user-data/outputs" rule was
implemented independently by the artifact editor and by IM-channel
attachment delivery, and the two copies had already drifted.
Move it into app/gateway/path_utils.py as normalize_outputs_virtual_path
(collapse `..` before the prefix check) and resolve_outputs_confined_path
(re-check the resolved host path against the resolved outputs root, which
also catches a symlink planted inside outputs/). PUT /artifacts and
ChannelManager._resolve_attachments both call the helper; artifact_archive
keeps its stricter ZIP-member rules layered on top.
Tests that previously stubbed resolve_thread_virtual_path for the editor now
stub resolve_outputs_confined_path, and the channel attachment tests patch
path_utils.get_paths, which the helper binds at import like the other
consumers. The confinement itself is pinned by tests/test_gateway_path_utils.py.
|
||
|
Willem Jiang
|
062273f850
|
chore(doc): update the CHANGLOG with the latest changes. (#5297)
* chore(doc):updated the CHANGELOG.md with latest changes * chore(doc):updated the CHANGELOG_zh.md with latest changes |
||
|
Willem Jiang
|
755b328caa
|
chore(doc):update the CHANGLOG and CHANGLOG_zh with latest changes (#5138)
* chore(doc):update the CHANGLOG with latest changes * chore(doc):update the CHANGLOG_zh.md with the change of CHANGLOG.md |
||
|
Willem Jiang
|
2a261d2276
|
chore(doc): update the CHANGLOG with the latest change in main branch (#5004)
* Update the CHANGELOG with latest changes * update the Chinese version of CHANGELOG |
||
|
Willem Jiang
|
26ba0b9e6a
|
doc(changelog): update the changelog with the latest status of 2.1.0 PRs (#4484)
* doc(changelog): update the change log files with latest PR status in mile-stone 2.1.0 * Added the chinese version changelog update |
||
|
Willem Jiang
|
98127f5845
|
Prepare 2.0.0 release (#3603)
* bump the version of deer-flow to 2.0.0 * Added CHANGELOG to the release branch * Update the changelogs files with the latest changes |