3 Commits

Author SHA1 Message Date
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
2026-09-06 09:01:33 +08:00
starslittle
f0276c9f5a
fix(memory): validate Honcho timeout and character limits (#4783)
* fix(memory): validate Honcho timeout and character limits

* fix(memory): enforce HonchoConfig invariants
2026-08-17 08:22:11 +08:00
ajayr
6cbf20fd39
feat(memory): add Honcho backend (user-model memory provider) (#4730)
* feat(memory): honcho backend config parsing

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* feat(memory): honcho v3 http client

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* feat(memory): honcho memory manager (workspace-per-user, fail-closed identity, async offload)

- HonchoMemoryManager implements the MemoryManager contract (add/get_context/
  search/get_memory/shutdown_flush + aadd/aget_context/asearch offloaded via
  asyncio.to_thread), signatures verified against manager.py's tier-1/tier-2/
  async abstracts.
- Workspace resolution: workspace_overrides[user_id] else
  workspace_prefix + sanitize_id(user_id); missing/empty user_id fails closed
  (no-op write, empty read) rather than falling back to a shared workspace.
  User peer: user_peer_overrides[user_id] else sanitize_id(user_id).
- get_context self-truncates to max_injection_chars and raises
  MemoryManagerError only under failure_policy.read=fail_closed; default is
  log-and-return "".
- Restore backends/honcho/__init__.py to the noop direct-import convention
  (MANAGER_CLASS = HonchoMemoryManager) now that honcho_manager.py exists,
  replacing Task 10's temporary lazy __getattr__ scaffold.
- Fix Task 10 deferred docstring minor: sanitize_id docstring now states the
  grammar allows up to 100 chars while this helper caps at 64.
- 19 new tests appended to test_honcho_memory_backend.py (write/read/async/
  lifecycle/factory-discovery); 27/27 pass. Verified end-to-end that
  manager.py's drop-in backend scanner resolves "honcho" with no core edits.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(memory): collision-resistant identity derivation, exception containment, passive-writes flag

Task review findings (2 Critical + 1 Important), all fixed in the same worktree:

- CRITICAL (cross-user bleed): sanitize_id is lossy -- "user.name@example.com"
  and "user-name@example.com" both sanitized to the same string, merging two
  users' memory into one workspace/peer. Add _stable_id() (sanitize_id output
  + 8-hex-char SHA-256 suffix of the raw id) and use it on the default
  (non-override) path in _workspace/_user_peer; workspace_overrides /
  user_peer_overrides still match on the raw key, unchanged. The hash suffix
  also guarantees a non-empty result for a raw id that sanitizes to "" (e.g.
  "!!!"), so _user_peer can no longer return "". Documented in the manager's
  isolation docstring.

- CRITICAL (exception containment): client.py's _post() called response.json()
  outside the try block, so a 200 with a non-JSON body raised a bare
  JSONDecodeError that would escape add() with no upstream handler. Wrap the
  parse and raise HonchoRequestError (mirrors Mem0Client._request). Broadened
  the manager's four boundary excepts from `except HonchoRequestError` to
  `except Exception` (mirrors openviking_manager.py's broad-guard precedent),
  with `except MemoryManagerError: raise` first so a contract error is never
  swallowed or double-wrapped.

- IMPORTANT: added requires_passive_writes_in_tool_mode: ClassVar[bool] = True
  -- Honcho's only write path is passive add() (no fact CRUD hooks), so tool
  mode must keep MemoryMiddleware writes flowing to the deriver. Mirrors
  mem0_manager.py's identical flag/rationale.

Minors addressed: get_memory(user_id=None) empty-shape-with-no-calls test;
empty-string user_id tests for add()/get_context(); dedicated collision test
proving two colliding raw ids resolve to different workspaces/peers.

10 new tests (37/37 total pass); RED verified by stashing only the
implementation files (tests import the not-yet-existing _stable_id, so the
whole module fails to collect) before restoring the fix.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* test(memory): blocking-io anchor for honcho backend; docs + config example

- Adds test_honcho_memory_backend.py in tests/blocking_io/ with fake-client blocking IO
- Mirrors openviking anchor structure and conftest conventions
- Updates backends/README.md with honcho row and config keys section
- Updates config.example.yaml with honcho commented block
- Updates backend/AGENTS.md with honcho memory backend bullet
  - Documents workspace resolution (prefix + collision-resistant sanitized id)
  - Documents tool mode passive write retention via MemoryMiddleware
  - Documents async entrypoint offloading via asyncio.to_thread
  - Documents fail_closed vs fail_open recall failure policy

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(memory): wire close() to shutdown hook; correct honcho README defaults and tool-mode note

- HonchoMemoryManager.close() releases the HTTP client, mirroring
  mem0_manager.py's pattern and the base MemoryManager.close() shutdown hook.
- README: fix workspace_prefix (deerflow-u-), message_char_limit (8000),
  max_injection_chars (6000), and base_url (default http://localhost:8000,
  not required) against backends/honcho/config.py; add missing
  timeout_seconds/connect_timeout_seconds rows; replace the "middleware
  mode only" claim with wording matching reality (tool mode supported,
  search implemented, passive writes retained via MemoryMiddleware).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(memory): honor failure_policy.read on all honcho recall paths; review nits

Addresses PR #4730 review feedback:
- search() and get_memory() now route through a _read_or_fallback policy
  gate (mem0's pattern), so failure_policy.read: fail_closed raises
  MemoryManagerError on every recall path as documented; get_context()
  uses the same helper, preventing future drift.
- Session ids use the collision-resistant _stable_id derivation; bare
  sanitize_id would merge threads like "t.1"/"t-1" into one session.
- HonchoClient accepts a transport kwarg (Mem0Client precedent) so tests
  inject httpx.MockTransport through the constructor.
- Config: empty/null workspace/peer override values fail fast at parse
  time instead of silently falling through to the default derivation.
- _UTC_NOW_FIELDS 1-tuple replaced by a plain _UTC_NOW_FORMAT constant.
- README: user_peer_overrides row described the wrong target (it
  overrides the user's own peer, not assistant_peer); document the
  non-empty constraint on override values.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* docs(memory): qualify honcho isolation claim for shared workspace_overrides

The module docstring claimed users cannot see each other's memory by
construction, unconditionally. That holds for the default
one-workspace-per-user derivation, but a workspace_overrides entry mapping
several users to one workspace shares that workspace's search index:
search() uses Honcho's workspace-scoped /search (no peer filter), while
get_context()/get_memory() stay peer-scoped via working_representation.
State the asymmetry in the docstring, the README Workspace Resolution
section, and the workspace_overrides table row.

Docs-only; no behavior change (review follow-up on #4730).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-08-12 09:02:08 +08:00