mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-09-10 05:58:36 +00:00
fix(memory): validate Honcho timeout and character limits (#4783)
* fix(memory): validate Honcho timeout and character limits * fix(memory): enforce HonchoConfig invariants
This commit is contained in:
parent
5ffaa09f5a
commit
f0276c9f5a
@ -53,6 +53,7 @@
|
||||
query-aware refresh policy and new lifecycle scheduling are separate changes,
|
||||
not part of this backend.
|
||||
- The optional `honcho` backend under `packages/harness/deerflow/agents/memory/backends/honcho/` is a remote-only HTTP adapter for user-model memory (RFC #1898's user-dimension option). Select with `memory.manager_class: honcho`, keep `memory.mode: middleware` (tool mode also supported — it implements `search`). It writes filtered turns as Honcho messages (no local LLM calls; Honcho's deriver builds representations server-side), resolves one workspace per `user_id` (`workspace_overrides` else `workspace_prefix + collision-resistant sanitized id`; missing user fails closed to no memory), offloads sync HTTP in its `a*` overrides via `asyncio.to_thread`, and tool mode retains passive writes via MemoryMiddleware, mirroring mem0. `failure_policy.read: fail_closed` rethrows recall failures; default is log-and-empty.
|
||||
- Honcho configuration objects reject non-finite or non-positive timeout values and non-positive character budgets during construction, including direct dataclass construction, before an HTTP client can use them.
|
||||
- `memory.mode: tool` skips `MemoryMiddleware` and registers `memory_search`, `memory_add`, `memory_update`, and `memory_delete` on the agent. The model decides when to search, add, update, or delete facts; this is opt-in/experimental and should not be described as better than middleware mode without eval evidence.
|
||||
- Both modes share `FileMemoryStorage`, per-user/per-agent isolation, manual CRUD primitives, and the updater backend. Injection is mode-aware: middleware mode injects global `user`/`history` summaries plus the selected agent's facts, while tool mode injects only the global summaries and leaves every agent fact behind `memory_search` to avoid duplicating automatically injected and retrieval-returned context. `memory.injection_enabled: false` suppresses the complete block in either mode.
|
||||
- Middleware extraction classifies proposed facts with extraction-only `scope`/`durability`/`authority` labels. `_apply_updates` accepts only `user` + `durable` + `descriptive` new/consolidated facts, accepts only wholly user-scoped summary prose with `authority=descriptive`, and rejects missing labels per item without aborting unrelated updates. Contradiction removals use object entries with `id`, `scope`, `reason`, and optional zero-based `replacementFactIndex`; task/project removals fail closed, and a paired removal runs only when the referenced replacement survives the scope/confidence gates, deduplication, and max-fact trim under another fact ID. The labels are not persisted, so no storage migration is required. Staleness removals retain their independent candidate/cap guardrails, while tool-mode CRUD remains outside this extraction gate. Custom `memory.backend_config.prompts_dir` templates (including per-agent overrides) must carry the same classification fields; an un-migrated template makes the fail-closed gate reject every extraction-driven write, observable only through `rejected_by_scope_gate` and the >60% fact-rejection warning.
|
||||
|
||||
@ -139,14 +139,14 @@ The optional `honcho/` backend is a remote-only HTTP adapter for user-model memo
|
||||
| `base_url` | str | `http://localhost:8000` | Honcho instance URL (e.g., `http://localhost:8000` or `https://api.honcho.dev`) |
|
||||
| `api_key` | str | optional | API key for hosted Honcho; required if `base_url` is `api.honcho.dev`. Can use `$HONCHO_API_KEY` env var syntax. Requires `allow_insecure_http: true` when using plain HTTP |
|
||||
| `allow_insecure_http` | bool | false | Allow HTTP (non-HTTPS) connections; needed for localhost development with api_key |
|
||||
| `timeout_seconds` | float | `10.0` | HTTP client timeout (seconds) for calls to Honcho — read/write/pool; see `connect_timeout_seconds` for the connect phase |
|
||||
| `connect_timeout_seconds` | float | `3.0` | HTTP connect timeout (seconds) for establishing the connection to Honcho |
|
||||
| `timeout_seconds` | float | `10.0` | HTTP client timeout (seconds) for calls to Honcho — read/write/pool; see `connect_timeout_seconds` for the connect phase. Must be finite and `> 0` |
|
||||
| `connect_timeout_seconds` | float | `3.0` | HTTP connect timeout (seconds) for establishing the connection to Honcho. Must be finite and `> 0` |
|
||||
| `workspace_prefix` | str | `deerflow-u-` | Prefix for isolated workspaces; each user gets one workspace named `{prefix}{sanitized_id}` |
|
||||
| `workspace_overrides` | dict | `{}` | Map specific user ids to custom workspace names; overrides the prefix-based derivation. Values must be non-empty (parse error otherwise). Mapping several users to one workspace shares its search index across them (see Workspace Resolution) |
|
||||
| `user_peer_overrides` | dict | `{}` | Map specific user ids to custom names for the user's own peer; overrides the stable-id derivation. Values must be non-empty (parse error otherwise) |
|
||||
| `assistant_peer` | str | `deerflow` | Default peer name for the assistant when storing messages |
|
||||
| `message_char_limit` | int | `8000` | Character limit per message; longer messages are truncated |
|
||||
| `max_injection_chars` | int | `6000` | Character limit for injected memory into the system prompt |
|
||||
| `message_char_limit` | int | `8000` | Character limit per message; longer messages are truncated. Must be `> 0` (zero empties the write; a negative value is a Python suffix slice, not a cap) |
|
||||
| `max_injection_chars` | int | `6000` | Character limit for injected memory into the system prompt. Must be `> 0` |
|
||||
| `failure_policy.read` | str | `fail_open` | Recall failure handling: `fail_open` (log and return empty) or `fail_closed` (rethrow) |
|
||||
|
||||
**Workspace Resolution**: Each DeerFlow user maps to one Honcho workspace. The workspace name is derived as: `workspace_overrides[user_id]` (if present) else `workspace_prefix + sanitized_id`, where `sanitized_id` is a collision-resistant hash suffix (sanitize[:48]-sha256[:8]). Missing user fails closed to no memory. The default derivation is isolated per user; a `workspace_overrides` entry that maps several users to one workspace deliberately shares that workspace's **search index** across them (`search` uses Honcho's workspace-scoped `/search`, which has no peer filter), while `get_context` / `get_memory` remain peer-scoped.
|
||||
|
||||
@ -10,6 +10,7 @@ from __future__ import annotations
|
||||
|
||||
import re
|
||||
from dataclasses import dataclass, field
|
||||
from math import isfinite
|
||||
from typing import Any
|
||||
|
||||
_ID_RE = re.compile(r"[^a-zA-Z0-9_-]+")
|
||||
@ -49,6 +50,18 @@ class HonchoConfig:
|
||||
read_fail_closed: bool = False
|
||||
storage_path: str = ""
|
||||
|
||||
def __post_init__(self) -> None:
|
||||
if not isfinite(self.timeout_seconds) or self.timeout_seconds <= 0:
|
||||
raise ValueError("Honcho backend: timeout_seconds must be a finite value > 0")
|
||||
if not isfinite(self.connect_timeout_seconds) or self.connect_timeout_seconds <= 0:
|
||||
raise ValueError("Honcho backend: connect_timeout_seconds must be a finite value > 0")
|
||||
# add()/get_context() truncate with text[:n]. n <= 0 is empty (n == 0)
|
||||
# or a Python negative slice (n == -1 -> text[:-1]), not a length cap.
|
||||
if self.message_char_limit <= 0:
|
||||
raise ValueError("Honcho backend: message_char_limit must be > 0")
|
||||
if self.max_injection_chars <= 0:
|
||||
raise ValueError("Honcho backend: max_injection_chars must be > 0")
|
||||
|
||||
@classmethod
|
||||
def from_backend_config(cls, backend_config: dict[str, Any] | None) -> HonchoConfig:
|
||||
cfg = dict(backend_config or {})
|
||||
|
||||
@ -23,6 +23,8 @@ class TestHonchoConfig:
|
||||
assert cfg.workspace_overrides == {}
|
||||
assert cfg.user_peer_overrides == {}
|
||||
assert cfg.assistant_peer == "deerflow"
|
||||
assert cfg.timeout_seconds == 10.0
|
||||
assert cfg.connect_timeout_seconds == 3.0
|
||||
assert cfg.message_char_limit == 8000
|
||||
assert cfg.max_injection_chars == 6000
|
||||
assert cfg.read_fail_closed is False
|
||||
@ -69,6 +71,58 @@ class TestHonchoConfig:
|
||||
with pytest.raises(ValueError, match="user_peer_overrides"):
|
||||
HonchoConfig.from_backend_config({"user_peer_overrides": {"bob": " "}})
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("key", "value"),
|
||||
[
|
||||
pytest.param("timeout_seconds", 0, id="timeout-zero"),
|
||||
pytest.param("timeout_seconds", -1, id="timeout-negative"),
|
||||
pytest.param("timeout_seconds", float("nan"), id="timeout-nan"),
|
||||
pytest.param("timeout_seconds", float("inf"), id="timeout-inf"),
|
||||
pytest.param("timeout_seconds", float("-inf"), id="timeout-neg-inf"),
|
||||
pytest.param("connect_timeout_seconds", 0, id="connect-timeout-zero"),
|
||||
pytest.param("connect_timeout_seconds", -1, id="connect-timeout-negative"),
|
||||
pytest.param("connect_timeout_seconds", float("nan"), id="connect-timeout-nan"),
|
||||
pytest.param("connect_timeout_seconds", float("inf"), id="connect-timeout-inf"),
|
||||
pytest.param("connect_timeout_seconds", float("-inf"), id="connect-timeout-neg-inf"),
|
||||
],
|
||||
)
|
||||
def test_rejects_invalid_timeouts(self, key, value):
|
||||
with pytest.raises(ValueError, match=key):
|
||||
HonchoConfig.from_backend_config({key: value})
|
||||
|
||||
@pytest.mark.parametrize("key", ["message_char_limit", "max_injection_chars"])
|
||||
@pytest.mark.parametrize("value", [0, -1])
|
||||
def test_rejects_non_positive_character_limits(self, key, value):
|
||||
with pytest.raises(ValueError, match=key):
|
||||
HonchoConfig.from_backend_config({key: value})
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("key", "value"),
|
||||
[
|
||||
pytest.param("timeout_seconds", float("inf"), id="timeout"),
|
||||
pytest.param("connect_timeout_seconds", 0, id="connect-timeout"),
|
||||
pytest.param("message_char_limit", -1, id="message-limit"),
|
||||
pytest.param("max_injection_chars", 0, id="injection-limit"),
|
||||
],
|
||||
)
|
||||
def test_direct_construction_enforces_limits(self, key, value):
|
||||
with pytest.raises(ValueError, match=key):
|
||||
HonchoConfig(**{key: value})
|
||||
|
||||
def test_accepts_custom_positive_timeouts_and_character_limits(self):
|
||||
cfg = HonchoConfig.from_backend_config(
|
||||
{
|
||||
"timeout_seconds": 2.5,
|
||||
"connect_timeout_seconds": 0.5,
|
||||
"message_char_limit": 12,
|
||||
"max_injection_chars": 8,
|
||||
}
|
||||
)
|
||||
assert cfg.timeout_seconds == 2.5
|
||||
assert cfg.connect_timeout_seconds == 0.5
|
||||
assert cfg.message_char_limit == 12
|
||||
assert cfg.max_injection_chars == 8
|
||||
|
||||
|
||||
class TestSanitizeId:
|
||||
def test_passthrough_and_cleanup(self):
|
||||
@ -271,6 +325,21 @@ class TestHonchoManagerWrite:
|
||||
sent = [c for c in fake.calls if c[0] == "messages"][0][1][2][0][1]
|
||||
assert len(sent) == 10
|
||||
|
||||
def test_from_config_rejects_negative_message_char_limit(self):
|
||||
"""add() uses ``text[:message_char_limit]``. A negative limit is a
|
||||
Python negative slice (``text[:-1]``), which deletes a suffix instead
|
||||
of capping length. Fail at config parse so the manager never sees it.
|
||||
"""
|
||||
with pytest.raises(ValueError, match="message_char_limit"):
|
||||
HonchoMemoryManager.from_config({"base_url": "http://honcho.test", "message_char_limit": -1})
|
||||
|
||||
def test_from_config_rejects_zero_max_injection_chars(self):
|
||||
"""get_context() uses ``representation[:max_injection_chars]``. Zero
|
||||
would inject an empty memory string; reject it at startup.
|
||||
"""
|
||||
with pytest.raises(ValueError, match="max_injection_chars"):
|
||||
HonchoMemoryManager.from_config({"base_url": "http://honcho.test", "max_injection_chars": 0})
|
||||
|
||||
def test_add_normalizes_list_content(self):
|
||||
mgr, fake = _manager()
|
||||
mgr.add("t-5", [_msg("human", [{"type": "text", "text": "part1"}, {"type": "text", "text": "part2"}])], user_id="u1")
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user