mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-11-02 00:20:40 +00:00
5 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Yufeng He
|
e325c90b24
|
fix(models): preserve and budget structured MindIE tool results (#6208)
* fix(models): preserve structured MindIE tool results Signed-off-by: lovett55 <xbzz101800@163.com> * fix(models): redact and bound structured MindIE tool results * fix(models): keep structured MindIE ToolMessage content visible _fix_messages flattened list content keeping only string and type=text blocks, so a JSON tool result became an empty <tool_response> and the model could not see the tool output on the next turn. Serialize type=json blocks onto the same text channel; the existing XML escaping applies to the serialized text unchanged. Fixes #6010 Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com> * fix(models): harden the MindIE json channel from review Review follow-ups on the structured-content fix: - json.dumps can also raise ValueError (circular reference); catch it alongside TypeError so the payload degrades to str() instead of failing the whole request normalization. - A {"type": "json"} block with no json key keeps the old drop behavior instead of emitting a literal "null" into the frame. - PiiRedactionMiddleware now redacts json block payloads leaf-wise (nested dict/list strings scanned, keys and non-string leaves kept), so the newly serialized channel participates in the configured redaction policy for user messages and remote tool results alike. Without it, redaction-enabled deployments would ship structured PII raw once the provider serializes the block. Tests: fallback degrade for both exception paths, missing-key drop, leaf-wise redaction on the model-call and tool boundaries, and a middleware-to-MindIE regression pinning redaction through provider serialization. Red/green verified: the 6 new redaction/json tests fail without the source changes, 155 pass with them. Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com> * fix(models): close MindIE structured tool redaction gaps * fix(agents): budget structured json tool output before provider normalization Willem's P2 on #6208: _message_text() returned None for {"type": "json"} blocks, so a large structured ToolMessage slipped past both the tool-call and history budget gates and reached MindIE's serializer at full size, ignoring the externalize threshold and the fallback cap. Render those blocks with the same json.dumps(..., ensure_ascii=False) serialization the MindIE adapter applies (str() fallback included), so they count toward the budget everywhere _message_text is consulted. Images and other non-textual blocks still skip enforcement. Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com> * fix(mindie): render json blocks for tool messages only InputSanitizationMiddleware neutralizes strings and text blocks, so a {"type": "json"} block smuggled into a genuine user message reaches _fix_messages with its payload untouched. Rendering it into the text channel handed the model an unescaped, unframed injection (e.g. a literal <system-reminder>), bypassing the guardrail entirely. Gate the json-block serialization on ToolMessage: structured tool results keep riding the text channel (and still get html.escape plus the <tool_response> framing), while user/assistant json blocks go back to the old drop behavior. Adds unit coverage for the drop paths plus a chain regression that runs a genuine user message through the sanitization middleware and then _fix_messages, asserting the smuggled payload never reaches the model-bound text. Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com> * fix(models): preserve configured budgets in combined MindIE fix --------- Signed-off-by: lovett55 <xbzz101800@163.com> Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com> Co-authored-by: lovett55 <xbzz101800@163.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
yetuge
|
34a105c084
|
fix(middleware): preserve message fields when PII redaction rebuilds a user message (#5968)
* fix(middleware): preserve message fields when PII redaction rebuilds a user message PiiRedactionMiddleware._process_request rebuilt the redacted HumanMessage with a hand-constructed constructor call copying only content, id, name and additional_kwargs, silently dropping response_metadata and every other field of the original. That is the third and last site of the family already fixed for ThreadDataMiddleware (#5823) and InputSanitizationMiddleware (#5875); the review on #5823 scoped the follow-up to the two input-sanitization sites and left this one out. The middleware was also inconsistent with itself: _redact_tool_message in the same file already rebuilds with model_copy and carries the comment explaining that a hand-built constructor call silently drops response_metadata. The user message now rebuilds with msg.model_copy(update={...}). The content is deep-copied because _redact_content appends untouched content blocks by reference: without the copy, mutating a non-text block on the redacted message would write through to the message retained in thread state. Red/green: test_redacted_user_message_preserves_other_fields asserts response_metadata/id/name/additional_kwargs survive the rewrite — red on main (response_metadata == {}), green here. A second test guards the aliasing behaviour above and goes red if the deep copy is dropped. Both suites 258 passed; ruff 0.15.12 check + format clean repo-wide. Fixes #5967 Signed-off-by: yetuge <2219677952@qq.com> * fix(middleware): keep the PII rewrite on copy failure and detach metadata Addresses the two copy-boundary findings on the previous commit. 1. A content block may carry a value whose copy raises. deepcopy(content) then raised out of _process_request, _try_process fell back to the original request, and the handler received raw PII unredacted. Copying now falls back to a shallow per-container copy, so a copy failure still produces a redacted message and never bypasses redaction. 2. msg.model_copy(update=...) is shallow, so the rebuilt message and the original shared one response_metadata dict: mutating metadata on the model-facing message wrote through to the request message retained in thread state. response_metadata is now copied independently. Red/green: test_preserved_metadata_is_not_shared_with_the_original and test_redaction_survives_a_content_block_that_cannot_be_copied both fail on the reviewed commit (the second with the raw address reaching the handler) and pass here. Suites: pii 77 passed, and pii + input_sanitization + uploads 327 passed. ruff check + format --check clean repo-wide (1694 files). Also drops the "third and last site" wording from the previous commit message's claim: UploadsMiddleware is a further site of the same pattern (#5972). Signed-off-by: yetuge <2219677952@qq.com> --------- Signed-off-by: yetuge <2219677952@qq.com> |
||
|
Totoro
|
8190c86280
|
fix(goal): show the active goal to the agent on every model call (#5917)
* fix(goal): show the active goal to the agent on every model call Nothing put ThreadState.goal into model requests. The agent only knew the goal from messages: the objective the Web UI and IM channels send, and the hidden <goal_continuation> message. Goals set through PUT /goal, the embedded client or the TUI never reached it, and compaction could remove the messages that stated the others. DurableContextMiddleware now renders the active objective first in its hidden durable-context data message, escaped, bounded, PII-redacted and treated as data like the summary. Only the objective is rendered, so the block changes only when the goal is set or cleared. * fix(goal): let the agent pursue the active goal at user priority The durable-context contract told the model never to follow instructions in its data message, and that message now carried the goal. While a goal is rendered, the contract ends with a static exception for the <active_goal> element that opens the data message: work toward it as a request in a user message, with no system or developer authority, while every other field value, and any other text that calls itself a goal, stays data. The element replaces the heading the goal used, which a summary could reproduce. Other field values in the data message are HTML-escaped, and active_goal joins the sanitizer denylist, so user input and remote tool results cannot carry a raw one either. --------- Co-authored-by: Totoro-qaq <279883115+Totoro-qaq@users.noreply.github.com> |
||
|
xiaodu55
|
64e499be98
|
feat(memory): redact queued memory payloads (pii_redaction follow-up) (#5577)
* feat(memory): redact queued memory payloads (pii_redaction follow-up) Closes the remaining vector from #3190 (memory storage): request-scoped redaction leaves originals in thread state, and MemoryMiddleware queued that raw conversation into the durable extraction buffer — so the extraction model's input and the persisted facts carried raw PII even with pii_redaction.enabled. - MemoryMiddleware gains pii_redaction_config (same pattern as summarization / durable-context / title); the payload returned by _resolve_add_args() is redacted at the enqueue boundary before add()/aadd(), covering both after_agent and aafter_agent - Reuses the shared redact_text() seam from #5527 via active_pii_detectors/_Redactor/_redact_content; the buffered payload itself becomes clean, not just the extraction model's view - Wired at the lead-agent assembly site - Tests: queue-payload redaction enabled/disabled, detector toggles, message immutability, token stability across turns * fix(memory): reserve existing placeholder indices across the queued batch Review round 1 on #5577: thread state mixes raw user turns with already-redacted tool results and summaries, so a token in a later message must not collide with a new value in an earlier one. Pre-scan the whole queued batch with redactor.reserve() before allocating. Also extend the two MemoryMiddleware stubs in test_lead_agent_model_resolution.py to accept the new pii_redaction_config kwarg (CI backend-unit-tests red). * fix(memory): redact the compaction-triggered memory_flush_hook queue too Review round 2 on #5577: with memory + summarization enabled, memory_flush_hook() queues list(event.messages_to_summarize) directly into MemoryManager.add_nowait(), bypassing MemoryMiddleware entirely — and compaction removes those messages from state right after, so the after-agent redaction can never repair a raw batch queued there. Extract the batch redaction (reserve pre-scan + rebuild) into a shared redact_queued_messages() used by both MemoryMiddleware's enqueue boundary and the hook; the hook gains an optional pii_redaction_config wired via functools.partial at the summarization factory (subagents use skip_memory_flush=True and are unaffected). Regression covers the hook path enabled/disabled/absent. * test(summarization): unwrap the partial in the factory hook identity check edd916e wrapped memory_flush_hook in functools.partial to carry the pii_redaction config, but missed committing the matching update to test_factory_attaches_memory_flush_hook_by_default — a functools.partial is not equal to its wrapped function, so the identity assertion failed deterministically (caught by static review on #5577). * fix(memory): cover tool-call arguments and lenient text blocks in queued payloads Review round 4 on #5577 — two reproduced gaps still allowed raw PII into memory payloads with pii_redaction.enabled=true: - OpenViking retains the full message object, so parsed tool_calls args and provider-format arguments in additional_kwargs bypassed the content-only redaction; queued payloads now redact tool-call args recursively (and reserve them in the pre-scan). - DeerMem's format_conversation_for_update reads p.get("text") of any dict block, not only type=="text" — _redact_content now redacts every string-valued text field so a lenient consumer cannot bypass it. Both repro shapes are pinned as regressions. * fix(pii): value-derived placeholders for stable capture identity Review round 5 on #5577: sequential allocation is order-dependent — the same unchanged message re-redacted in a later batch can get a different token, which flips OpenViking's content-signature and replays already recorded turns into persistent memory sessions. Placeholders are now a pure function of the matched value (sha256-6 per category): identities stay stable across batches, enqueues, seams, and backends with no shared state and no stored mapping. - _Redactor becomes stateless; the reserve machinery (index pre-scans, _PLACEHOLDER_PATTERN) is removed along with its call sites - redact_texts keeps its signature, now a simple per-text redaction - reserve-semantics tests are superseded by cross-enqueue identity tests (two successive captures where the second batch carries higher existing placeholders); the compiled-graph echo regex matches the new format * fix(pii): 128-bit placeholder digests; redact tool-argument keys Review round 6 on #5577: - [P2] The 24-bit truncation collided distinct identities (reproduced: contact3513@ / contact3727@example.com both rendered [EMAIL_575dbb]). Retain 128 bits of the digest so separate people stay separate at any realistic volume; the compiled-graph echo regex and the AGENTS.md entry follow the new format. - [P2] _redact_strings redacted values but preserved mapping keys, so {"contacts": {"alice@example.com": "manager"}} kept the raw email as a key. Redact string keys along with values; deterministic 128-bit tokens make distinct-key collisions a hash-collision non-event, and a redacted key equal to another key merges the entries. Regression pins the email-keyed arguments shape. * docs(agents): trim PiiRedactionMiddleware entry; drop stale reserve wording The chain grew past the 98304-byte AG002 hard limit again (main growth plus the round-6 format example). Trim the entry to fit, and replace the stale reserve-indices wording with the value-derived semantics that replaced it. * docs(agents): shrink PiiRedactionMiddleware entry below the chain budget main moved (#5609) and the base comparison flipped the nearly-over chain back to an AG002 error (98441 > 98304). Trim the entry to the facts that are not documented elsewhere: seams, allowlist, order, enqueue boundary. * fix(pii): letters-only value-derived tokens; wire the factory SDK path Review round 7 on #5577: - [P2] Hex digests carry digit runs that later digit-anchored detectors re-scan, corrupting minted tokens into nested placeholders (reproduced: user280@example.com -> [EMAIL_…[PHONE_…]…], ~1/140 emails). The 128-bit digest is now encoded over a-z, so a minted token survives the full pinned detector order. - [P2] create_deerflow_agent (the SDK path) constructed MemoryMiddleware and DurableContextMiddleware without pii_redaction_config; the knob is now an explicit optional parameter threaded through _assemble_from_features (the factory reads no config files by design, so SDK callers pass it explicitly). - [Nit] hoist the Command rebuild comprehension out of the per-message loop in _redact_result. - [Nit] drop the dead _iter_tool_calls helper and Iterator import. * test(factory): pin pii_redaction_config wiring; document the parameter Review round 8 on #5577: - [P3] test_create_deerflow_agent.py had zero coverage of the pii_redaction_config parameter; two regressions now assert that create_deerflow_agent(pii_redaction_config=cfg) builds a MemoryMiddleware and a DurableContextMiddleware carrying cfg, and that the default is None (redaction off). - [Nit] document the parameter in the create_deerflow_agent docstring: None leaves redaction off; a middleware= takeover bypasses the assembly that would apply it. * fix(pii): HMAC-scoped placeholder tokens; install model-bound redaction in the SDK chain Review round 10 on #5577: - [P2] sha256(category||value) is an unkeyed, globally deterministic fingerprint: a provider or memory backend can confirm candidate values offline and link identical values across users and threads, contradicting the documented no-re-identification guarantee. Placeholders are now HMAC-derived over a deployment-scoped token_secret (new config field); the docstring documents the linkability scope for both modes. - [P2] _assemble_from_features() never appended PiiRedactionMiddleware, so the SDK knob enabled only the downstream seams while the primary model still received raw content. The middleware is now appended in the same relative position as the lead-agent base chain when the gate is on, with a model-call regression through the assembled SDK chain. * docs(pii): document token_secret; update the placeholder format examples Follow-up to the round-10 review on #5577: - config.example.yaml documents the new token_secret knob: a deployment-scoped secret makes placeholders HMAC-derived and only linkable within one deployment; leaving it unset keeps publicly computable fingerprints (documented trade). - the PiiRedactionConfig class docstring's placeholder example now matches the shipped format (128-bit HMAC digests over base-26 letters). * docs(pii): update the module docstring's placeholder format example The module docstring still showed the sequential [EMAIL_1] example; align it with the shipped value-derived format (128-bit HMAC digests over base-26 letters). * fix(pii): require a non-empty token_secret when redaction is enabled Review round 11 on #5577: with the secret optional, the default enabled configuration minted empty-key HMAC digests — publicly computable, globally linkable fingerprints of the raw values, confirmable by offline guessing for low-entropy identifiers such as phone numbers. - PiiRedactionConfig now rejects enabled=true without a non-empty token_secret (minimum 16 characters); the unkeyed mode is gone entirely. - The request-scoped redactor in _process_request now carries the configured key: it built its redactor without it, so user-message placeholders silently diverged from every other seam for the same value. The empty-key guard in _placeholder_token is the backstop that keeps every path keyed. - Module/config docstrings and config.example.yaml no longer describe the placeholders as irreversible or the secret as optional; the stale reserve-semantics wording and the "memory path not covered" note are corrected. - Regressions: validation errors for missing/blank/short secrets, and the user-message boundary minting the same keyed token as redact_text. * fix(memory): redact original_user_content on queued memory payloads Review round 12 on #5577: UploadsMiddleware preserves the raw user turn in additional_kwargs.original_user_content, but redact_queued_messages rewrote only content, tool_calls, and provider-format tool_calls — the queued copy still carried the raw value, so the memory queue and backend received raw PII with redaction enabled. The shared helper now redacts the provenance string field on the queued copy (same batch redactor, merged with the existing additional_kwargs rewrite, rebuilt via model_copy — thread-state messages stay untouched), covering both the enqueue boundary and the compaction flush hook. Regressions pin the redacted provenance field through both paths and the untouched original. * fix(memory): redact human_input_response on queued memory payloads Review round 13 on #5577: clarification replies keep the raw user answer in additional_kwargs.human_input_response.value (DeerMem reads that mapping via its read_human_input_response mirror), but redact_queued_messages rewrote only content, tool_calls, and original_user_content — the queued copy still carried the raw value into the memory queue and backend. The shared helper now rebuilds that mapping with value redacted (same batch redactor) while preserving the protocol fields (version/kind/source/ request_id/option_id/response_kind), merged with the existing additional_kwargs rewrite and rebuilt via model_copy so thread-state messages stay untouched. Regressions pin the redacted value, preserved protocol fields, the untouched original, and the flush-hook path. * chore(config): bump config_version to 47; generate token_secret on upgrade Review round 13 on #5577: existing deployments can legitimately sit at config_version 46 with pii_redaction.enabled: true and no token_secret; the mandatory-secret validator would fail their startup while config.example.yaml still said 46, so the upgrade checker considered them current and emitted no migration warning. - Bump config_version to 47 so the outdated-config warning fires for v46. - Add a v47 migration to scripts/config-upgrade.sh: when pii_redaction is enabled and token_secret is missing/blank, generate a random deployment- scoped secret (secrets.token_urlsafe) and persist it, so the upgraded deployment starts under the mandatory validation. - Regressions: a v46 enabled config is flagged outdated against the real example; the upgrade run generates a >= 16-char secret and the result passes AppConfig validation; a disabled config gets no secret. - Also switch the two v41 script tests from a hardcoded "bash" to SCRIPT_BASH/skipif, matching the documented Windows constraint and the neighboring tests (hardcoded bash resolves to the WSL stub there). * chore(helm): sync chart config_version to 47 Review round 14 on #5577: check_config_version.sh fails whenever the chart's embedded config_version lags config.example.yaml, and the chart workflow is triggered by config.example.yaml — so the v47 bump left this head failing that check. Update the values.yaml config block and the README example to 47; scripts/check_config_version.sh now passes. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
xiaodu55
|
2ff006b0c0
|
feat(middleware): add deterministic PII redaction for model-bound context (#5527)
* feat(middleware): add deterministic PII redaction for model-bound context * fix(middleware): claim national IDs before cards, redact Command results, preserve ToolMessage fields - Reorder detectors so checksum-gated national IDs run before the credit-card detector; an 18-digit resident ID whose digit run also passes Luhn is no longer mislabeled [CREDIT_CARD_n] (review finding, reproduced at 0a2a9d0) - Redact ToolMessages carried in Command.update.messages, mirroring ToolResultSanitizationMiddleware's dc_replace pattern - Rebuild redacted ToolMessages via model_copy so artifact and response_metadata survive - Extend the numbered middleware chain in agents/middlewares/AGENTS.md * fix(middleware): span one redactor per Command result; refresh stale AGENTS.md entry range - Placeholder numbering now continues across every ToolMessage carried in a single Command result (one _Redactor per _redact_result call) instead of restarting per message - The renumbered AGENTS.md chain still referenced entries 9-12 in the ToolReceiptMiddleware entry; it now reads entries 10-13 * docs(agents): trim PiiRedactionMiddleware entry to fit the AGENTS.md chain budget The main merge (fb36e0e) pushed the effective middlewares chain to 98341 bytes, 37 over the 98304 hard limit checked by agent-guidance (AG002). Compress the entry while keeping the load-bearing facts: config gate, both interception points incl. Command coverage, detector order rationale, per-result numbering continuity, irreversibility, memory follow-up. * fix(middleware): redact compaction input and reinjected summaries; harden detectors Review round 3 on #5527: - [P1] SummarizationMiddleware invokes its summary model directly from before_model, outside PiiRedactionMiddleware's wrap_model_call, so raw thread state reached the summary model and reinjected summaries carried raw PII into model-bound context. Add a shared redact_text() seam: the compaction prompt is redacted in _build_summary_prompt (app_config already flows into the middleware) and DurableContextMiddleware redacts summary_text at reinjection via a new pii_redaction_config knob wired at both assembly sites. - [P2] CUIT is 2+8+1 digits, not 2+10+1. - [P2] Digit-anchored patterns use digit-aware lookarounds instead of Unicode \b, which CJK characters defeat (身份证110105… / 手机号138…). - [P2] The international-phone pattern no longer treats newlines as separators, so a candidate cannot swallow the following numeric field and then fail validation as a whole. * fix(pii): redact title input and reserve summary placeholders --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |