Commit Graph

69 Commits

Author SHA1 Message Date
ehz0ah
d57c2a3254 fix(memory): forward committed entry identity to providers 2026-09-23 01:06:32 -07:00
kshitijk4poor
28bd8cc08c fix(memory): never dedupe indented recall lines
Only column-0 bullets participate in `_drop_repeated_recall_lines`.
An indented line is a continuation of the bullet above it (nested
child, provenance, wrapped prose) and is kept verbatim, so identical
children under two different parents both survive. The docstring
already promised this; the code now matches.

PROOF: with the old code, `- Project A\n  - status: active\n- Project B\n
  - status: active\n` lost B's child (probe s4_eff_nested.py) and the
new assertion in test_a_bullet_with_continuation_lines_is_never_touched
fails (AssertionError at :49); after the change the probe returns the
input unchanged and tests/agent/test_memory_context_dedupe.py passes.
2026-09-22 15:55:09 +05:30
kshitijk4poor
8f618d8409 refactor(memory): match the recall bullet regex once per line
`_RECALL_BULLET_RE.match(stripped)` was evaluated twice per line and the inline
comment restated the docstring's per-section scoping paragraph. Compute
`is_bullet` once and keep a one-line comment. Behaviour-neutral.

PROOF: mutation `is_bullet = True` → tests/agent/test_memory_context_dedupe.py
1 failed (repeated-bullet), restored → all 44 tests across the PR's four test
files pass; ruff clean; footguns clean; real import OK.
2026-09-22 15:55:09 +05:30
kshitijk4poor
af0305c95a fix(memory): any column-0 non-bullet line opens a new dedupe section
The per-section reset only fired on `#…`, `**…` and `---` lines. The
in-tree RetainDB provider writes prose headings (`Profile:`,
`Relevant memories:`, `Instructions:`), so its second section still
collapsed into the first: `Profile:\n- None\nRelevant memories:\n- None`
lost the second `- None` and left a heading claiming nothing — the
exact failure 931d240e14 set out to fix, for markdown headings only.

Now any non-empty, non-bullet line at column 0 (a heading of any style,
a `---`/`***`/`___` rule, prose) clears `seen`; bullets and indented
continuation lines stay inside the current section.

PROOF: tests/agent/test_memory_context_dedupe.py::
test_identical_placeholder_bullets_under_different_headings_both_survive
(RetainDB prose-heading shape + markdown shape). Red with
agent/memory_manager.py at the previous commit (prose case drops the
second `- None`), green after; the continuation-line carve-out test
stays green.
2026-09-22 15:55:09 +05:30
kshitijk4poor
e1ea7e74f0 fix(memory): repeated-bullet dedupe is scoped per section
The `seen` set from #117399 was block-global, so identical placeholder
bullets under different headings ("- (none recorded)" under two stores)
collapsed into the first section and left the second heading claiming
nothing. Reset the set at every heading (`## …` / `**…**`, the PR's own
heading detection) and at `---`, so only a repeat inside the same
section is dropped.
2026-09-22 15:55:09 +05:30
John Paul Soliva
83a1a9a2bb fix(memory): only a self-contained bullet is deduped, and a bold heading is not a bullet
Review follow-up on two real defects in the first cut, both reproduced before fixing.

A dropped duplicate left its continuation lines behind, and they re-parented under the surviving
bullet: `- prefers draft PRs` / `  (logged 12 Jan, supermemory)` followed by the same headline
logged elsewhere collapsed into ONE entry carrying BOTH provenance lines — inventing a record
neither provider reported, then stamping it into `api_content` to be replayed every turn. That is
worse than the duplicate the change set out to remove. A bullet that carries continuation lines is
now never dropped and never suppresses a later one, so two entries that share a headline and differ
underneath it both survive.

And `stripped[:1] in ("-", "*")` treated `**Preferences**` as a list item, so a repeated bold
section heading was silently deleted — contradicting the docstring's promise that headings survive.
The marker test now requires a real bullet: marker, whitespace, content (`[-*+]\s+\S`), which also
keeps `*emphasis*` out. Numbered items stay out of scope, and the docstring says so rather than
claiming "list items" generally.

Measured again on the same 166 real blocks: still 219,260 of 1,302,220 body bytes — 16.8%,
unchanged. Every duplicate in that sample is a self-contained bullet, so correctness cost nothing.

Tests: a bullet with continuation lines is untouched; a nested bullet is a child, not a repeat;
same-depth indented duplicates still collapse; bold headings, emphasis and numbered items are left
as written. Removing the continuation guard fails two. The tautological assertion the review flagged
is replaced by an exact whole-body comparison.

(cherry picked from commit 37a36f23658b8ca1e112f46ff60a705a66f80a5b)
2026-09-22 15:55:09 +05:30
John Paul Soliva
cbe23de61a perf(memory): a recalled line is stated once per memory-context block
Every user turn injects a `<memory-context>` block built from the providers' prefetch, and nothing
dedupes it: a provider merges several stores and `prefetch_all` merges several providers, so one
prefetch routinely surfaces the same fact two or three times.

It is not paid once. The composed block is stamped into the user row's `api_content` sidecar and
replayed verbatim on every later request for as long as that row is in context — deliberately, so
the prompt-cache prefix stays byte-stable. Compaction cannot reclaim it either: `_demote_tool_result_at`
bails on anything that is not `role == "tool"`, so a user row is never shrunk and drags its memory
dump along while counting against the tail budget. A duplicated bullet therefore costs its tokens
once per turn, for the life of the row, while telling the model nothing that same block has not
already said.

Measured on a real operator state.db (166 rows carrying a block, 46 sessions): 219,260 of 1,302,220
body bytes — 16.8%, ~54,815 tokens on first send alone — are list items byte-identical to an earlier
line of the SAME block. Median block ~2,034 tokens per user turn.

Only list items are considered, and only when identical after stripping: headings, prose, blank
lines and `---` rules are left exactly as the provider wrote them, so sections still read as
written and a line repeated deliberately as prose is untouched. Within one block only — deduping
against earlier turns would save far more (73% of bullet bytes in that sample were already sent
earlier in the session) but changes what the model sees on a turn, so it is not done here.

The "provider returned pre-wrapped context" warning stays keyed on sanitization alone: a deduped
bullet is routine, not a provider fault.

Fixes #117397

(cherry picked from commit e3b08ca9b1708a7cddc2f1a2191b51d60d194624)
2026-09-22 15:55:09 +05:30
teknium1
9ba5850e95 refactor(memory-plugins): background threads inherit the profile context; JSON sidecar reads and the holographic config.yaml write use core primitives
Six of eight memory providers spawned plain threading.Thread for prefetch/sync/
writer work. A plain thread starts with an EMPTY contextvars.Context, so under
multiplex profiles the worker resolved the DEFAULT profile's HERMES_HOME (and
fails closed on scoped secrets). honcho and hindsight had each noticed and
written their own copy_context() wrapper; core had a third in memory_manager.
One canonical pair now lives on the ABC module every provider already imports:
agent/memory_provider.py::ctx_bound / spawn_context_thread. memory_manager,
honcho, hindsight, mem0, retaindb, byterover, supermemory and openviking all use
it; the honcho and hindsight wrappers and memory_manager._ctx_bound are deleted.

Five "json.loads(path.read_text()) or {}" readers (mem0._read_mem0_json,
honcho client/oauth/cli _read_config, hindsight save_config/_load_config) fold
into utils.read_json_or_empty, the read half of every read-merge-atomic_json_write
sidecar store.

holographic.save_config was the only config.yaml writer in the tree that
bypassed hermes_cli.config.save_config: raw open("w") + yaml.dump with no config
lock, no managed-mode refusal, no atomic replace, and a swallowed exception. It
now calls save_config(..., merge_existing=True). Behavior change: a managed
install refuses the write (previously silently rewrote config.yaml); other
sections are deep-merged instead of round-tripped through a raw dump.

openviking._hermes_home_path guarded an impossible ImportError of
hermes_constants (the module already imports agent.*) with a ~/.hermes fallback
that is wrong on Windows and under profile overrides; it is replaced by
get_hermes_home() directly.

Tests: tests/plugins/memory/test_provider_threads_inherit_profile.py drives each
provider's real spawn path with a fake backend and asserts the thread sees the
spawner's HERMES_HOME override (sabotage: retaindb back on threading.Thread ->
red). tests/plugins/memory/test_holographic_save_config.py pins merge-with-
existing-sections and managed-mode refusal (sabotage: raw yaml.dump -> red).
2026-09-13 05:19:48 -07:00
zhouhe-xydt
5084237f8e fix(memory): load provider schemas before mutating MemoryManager state
`add_provider()` flipped `_has_external` and appended the provider before
calling `get_tool_schemas()`. When schema loading raised, the broken provider
stayed registered and the single-external slot was poisoned for the rest of
the process: every later provider was rejected as "already registered".

Materialize the schema list first; state changes only after it succeeds.
Exception propagation is unchanged.

Hand-port of PR #9997 by @zhouhe-xydt onto the current add_provider() (the
reserved-core-tool filter landed in between); one invariant test.

Fixes #9948
2026-09-12 08:22:57 -07:00
Erosika
b41b036e64 fix(memory): pass on_turn_start kwargs only to providers that accept them
MemoryManager.on_turn_start forwarded the author kwargs to every provider and _each_provider swallowed the TypeError, so a provider with the two-positional on_turn_start(n, text) stopped running. The kwargs are now filtered against the provider's signature the way _provider_sync_accepts filters sync_turn.
2026-09-10 10:27:07 -07:00
Erosika
8969511209 feat(memory): carry the turn's author to sync_turn
on_turn_start already received the author trio. sync_turn did not, so a
provider that wanted to write the turn under its author had to stash state
between the two hooks. sync_turn now takes turn_author as a keyword-only
argument, and MemoryManager sends it only to providers whose signature
accepts it, so existing providers keep working unchanged.

build_turn_context resets the author on the agent at the start of every turn
so a cached gateway agent never carries a bot author into the next human
turn. agent/turn_author.py holds the parsing and the HERMES_TURN_AUTHOR
carrier.

MemoryProvider.identity_signature() is a new optional hook: the identity
values a provider writes under, declared by the provider itself, for the
gateway's agent cache to key on.
2026-09-10 10:27:07 -07:00
Teknium
4fbb253904 fix: mirror successful memory alias writes to providers 2026-09-07 06:05:13 -07:00
Teknium
2ed33fb38e refactor(memory): keep the spill, drop registration-trust rework
The prefetch spill is the fix; the is_builtin registration flag and duplicate-
instance rejection defended against a provider naming itself "builtin", which no
live path can do (only a test fixture does). Restore the name-based check and the
four test files that only changed for the new kwarg; keep two spill invariants.
2026-09-06 13:25:48 -07:00
Joey
d932fa5929 fix(memory): spill oversized external prefetch 2026-09-06 13:25:48 -07:00
Teknium
e83816a4d1 review-fix(comments): restore lost #NNNN rationale comments across non-test source (mechanical sweep, condensed, code unchanged)
For each issue anchor present in BASE 63279301bc non-test .py and absent on HEAD, the BASE comment/docstring block was re-attached at the HEAD location of the code it explained (matched by the distinctive code line / enclosing def). Sentences already covered by an existing HEAD comment were deduped; the issue number always survives. Insert-only: no code lines changed.
2026-09-03 09:44:26 -07:00
Teknium
5a17f15d86 refactor(agent): inline queue_prefetch fan-out lambda in memory_manager 2026-09-02 22:22:02 -07:00
Teknium
e74d8bbca6 refactor(agent): memory_manager pass-2 structural cut (902->802 LOC, zero behavior change)
- inline single-use helpers: _nonblank, _get_sync_executor (into _submit_background),
  _is_block_boundary/_max_pending_open_suffix (into _ends_at_block_boundary users),
  _is_checkpoint_provider (into on_pre_compress)
- unify _drain_sync_executor drained/timed_out state update into one path
- pack over-long signatures/_each_provider calls to <=120 cols
- collapse if/elif detail ladder, flush_pending except ladder, prefetch-thread guard
- compact docstrings that restated code (all WHY notes kept)

Verified: E corpus + extra corpus byte-identical vs base, 41 test files / 784 passed,
ruff clean, import smoke ok.
2026-09-02 22:06:35 -07:00
Teknium
3b5aa80473 refactor(agent): finish memory/compaction/prompt-cache compaction pass (>=25% LOC) 2026-09-02 19:50:13 -07:00
Teknium
9e1d3862ad refactor(agent): extract micro-compaction phase helpers, flatten memory-manager plumbing (pass 3) 2026-09-02 19:17:08 -07:00
Teknium
21cbb27d89 refactor(agent): compact memory/compaction/prompt-cache modules (pass 2, corpus parity) 2026-09-02 19:07:30 -07:00
Teknium
4f20954c5f refactor(agent): tighten memory/compaction/prompt-cache helpers (pass 1, corpus parity) 2026-09-02 18:28:38 -07:00
Teknium
8dcb2b6ada refactor(agent/providers): shared ProviderBase/CatalogProviderBase and provider_media; compact contract docs
- provider_base.py: ProviderBase (name/display_name/get_setup_schema) and
  CatalogProviderBase (default_model/list_models/is_available) replace the
  identical default-method bodies duplicated across 7 provider ABCs
- provider_media.py: one save_b64/save_bytes/save_url/cache_dir implementation
  behind image_gen_provider and video_gen_provider
- memory_manager.py: _each_provider fan-out helper replaces per-hook
  try/except loops; _signature_params/_has_var_kwargs unify signature probes
- image_routing.py: _resolve_inference_value shared by base_url/api_key
  resolution; _dict_or_empty/_clean_str/_custom_provider_entries helpers
- MemoryProvider/ContextEngine/TTS/browser/web/terminal-env ABC docstrings
  compacted to their invariants; method names and signatures unchanged
2026-09-02 13:53:28 -07:00
Teknium
8b73720fa7 fix(memory): tolerate bare-signature v2 providers when forwarding checkpoint requirement
Hardening on top of @Soju06's forwarding fix: v2 providers written against
the original docs example (def on_pre_compress(self, messages)) must not
TypeError when the host forwards require_checkpoint — inspect the signature
and fall back to the legacy call shape. Docs example updated to advertise
the keyword.
2026-08-31 09:57:22 -07:00
Soju06
5db9058cdb fix(memory): forward checkpoint requirement to v2 providers
MemoryManager.on_pre_compress() detects checkpoint API v2 providers,
selects the normalized evidence list for them, and re-raises their
failures under require_checkpoint — but it never tells the provider
that a checkpoint is required: the call passes only the messages.

A v2 provider therefore runs in its default best-effort mode, swallows
durable-write failures, and returns normally; the host then treats the
checkpoint as succeeded and lossy compression proceeds. With
compression.checkpoint_required: true this silently defeats the
guarantee the option exists to provide.

Forward require_checkpoint only to providers advertising the requested
checkpoint API version. Legacy providers keep the strict one-argument
on_pre_compress(self, messages) contract, so bundled v1 providers
(honcho, mem0, supermemory, ...) are unaffected.

Regression tests cover required and best-effort signaling, legacy
signature compatibility, and required-mode failure propagation.
2026-08-31 09:57:22 -07:00
Teknium
9e551d2931 refactor(memory): renumber checkpoint API — v1 is the implicit historical contract, v2 opts into fail-closed checkpoints
Per review: existing providers should not be retroactively re-versioned or
handed a changed payload. Version 1 is now the implicit historical
on_pre_compress() contract (best-effort, raw message list) that every
pre-existing provider is already on; the fail-closed checkpoint contract
becomes version 2. MemoryManager routes the raw transcript to v1 providers
unchanged and hands the host-normalized evidence list only to v2+ checkpoint
providers, so the plugin surface contract for shipped providers is
byte-identical with the gate off.
2026-08-25 03:55:55 -07:00
Jan-Stefan Janetzky
1104ffe0b9 feat(memory): opt-in fail-closed pre-compress checkpoint contract (API v1)
Context compression is intentionally lossy. Deployments that archive
transcript evidence to an external durable store before compaction had no
way to guarantee the archive actually happened: MemoryManager.on_pre_compress
swallows provider failures by design, so a failed archive silently degraded
into data loss.

This adds an opt-in, provider-agnostic checkpoint contract:

- memory_provider: PRE_COMPRESS_CHECKPOINT_API_VERSION = 1; providers opt in
  by advertising pre_compress_checkpoint_api_version. Version 0 keeps the
  historical best-effort hook semantics.
- memory_manager: supports_pre_compress_checkpoint() capability probe;
  on_pre_compress(require_checkpoint=True) propagates checkpoint-provider
  failures and raises when no capable provider completed the checkpoint.
- conversation_compression: new compression.checkpoint_required config key
  (default false, documented in cli-config.yaml.example). When enabled,
  compaction fails closed with BLOCKED_MISSING_PREREQUISITE (the
  uncompressed transcript is preserved) unless a checkpoint-capable provider
  confirms the durable checkpoint. Providers receive normalized direct
  user/assistant evidence: tool rows, system messages, tool-call wrappers,
  and prior compaction summaries are filtered host-side into one stable
  contract. codex_app_server compaction is rejected under the gate because
  it exposes no truthful pre-compaction transcript boundary.
- hermes_state: persistent _compressed_summary column (declarative schema
  migration via _reconcile_columns) so summary provenance survives process
  restarts; only the resume model history carries the marker, keeping
  get_messages_as_conversation on its existing contract.
- gateway: the lossy hygiene/auto-compact paths load the memory provider
  (skip_memory=False) so a required checkpoint also guards those rewrites.

The gate arms only on an explicit boolean True (bare-MagicMock agents in
existing tests have truthy auto-attributes). Default behavior is unchanged:
checkpoint_required=false preserves best-effort semantics for all existing
providers. Contract tests, including a restart round-trip of the summary
marker, in tests/agent/test_pre_compress_checkpoint_contract.py.

Refs #93986
2026-08-25 03:55:55 -07:00
Teknium
8172be0e8e fix(memory): log when a configured provider's tools are gated off by toolset config
Follow-up to the cherry-picked gate-parity fix: the silent 'return 0'
in inject_memory_provider_tools made #81014 undiagnosable — a
configured provider looked half-on with no hint which config key
suppressed its tools. Now an INFO line names the withheld providers
and the gating keys.
2026-08-24 21:45:30 -07:00
Chen Jin
b45b028573 fix(agent): gate memory provider system_prompt_block on toolset config (#81014)
The external memory provider's `system_prompt_block()` was injected
unconditionally into the system prompt, while the provider's tools were
gated by `memory_provider_tools_enabled()` via platform_toolsets or
disabled_toolsets. Result: the agent received instructions to call
`mnemosyne_remember`, `mnemosyne_recall`, etc., that did not exist in
its tool surface.

Centralize the gating into `memory_provider_tools_exposed(agent)`, use
it from both `inject_memory_provider_tools` and the system prompt
assembly path, and add regression tests covering:
* memory toolset enabled -> both tools and prompt block exposed,
* memory in disabled_toolsets -> neither exposed,
* memory not in enabled_toolsets and not built-in -> neither exposed,
* the built-in "memory" tool present as an opt-in -> both exposed,
* parity between `inject_memory_provider_tools` and
  `memory_provider_tools_exposed`.
2026-08-24 21:45:30 -07:00
kshitij
5118692c25 fix: replace double-lambda with functools.partial, close from_env config_path gap
- _submit_background and _prefetch_provider: replace unreadable
  (lambda inner: (lambda: ctx.run(inner)))(fn) with functools.partial(ctx.run, fn)
- from_env(): set config_path=resolve_config_path() so bound_config_path()
  doesn't re-resolve from ContextVar on daemon threads (the exact bug
  the PR fixes for from_global_config)

Review follow-ups for salvaged PR #83525.
2026-08-13 23:43:15 +05:30
Erosika
a5dde2d176 fix(memory): propagate contextvars through MemoryManager background lanes
MemoryManager dispatches provider sync_turn/queue_prefetch work on a
single-worker executor and hot prefetch on a plain thread. Neither
carried the caller's contextvars, so in multi-profile processes the
provider work ran outside the profile's ContextVar-scoped HERMES_HOME
override — any ambient resolution inside a provider landed on the
default profile.

Wrap the submitted callable and the prefetch thread target with
contextvars.copy_context().run, mirroring the gateway's
_run_in_executor_with_context pattern. Provider-agnostic: benefits
every external memory provider, not just Honcho.
2026-08-13 23:43:15 +05:30
kshitij
8b243dff62 fix: security + efficiency review fixes for salvaged PR #74379
1. Use open_credentialed_url() instead of bare urlopen() in
   templates.py apply_template() and probe_existing_customization().
   Both send Authorization: Bearer headers; bare urlopen forwards
   credentials on cross-origin redirects. The codebase has
   open_credentialed_url() in hermes_cli/urllib_security.py that
   strips credentials on cross-origin redirects — used by 4 other
   modules.

2. Guard unavailable_reason() with the dedup set check before
   calling it. The gateway builds a fresh AIAgent per message, so
   without this guard unavailable_reason() (which calls _load_config()
   → stat + file read + JSON parse, and _check_local_runtime() →
   importlib probes) runs on every gateway turn for an unavailable
   provider, even though the warning is deduped after the first.

3. Move INDICATOR_GLYPH from Hindsight's eye emoji to a generic
   brain (🧠) in core (agent/memory_provider.py). Hindsight overrides
   with its own _HINDSIGHT_GLYPH (👁️) in recall_status() and
   _emit_saving_indicator(). Other memory providers no longer inherit
   Hindsight's brand mark as the default glyph.
2026-08-13 23:15:25 +05:30
Ben
34c727c5c2 feat(hindsight): memory provider improvements — recall_sync, retain_source, setup templates, memory indicators, error hints
Bundles previously-separate Hindsight/memory PRs into a single review surface:
- opt-in synchronous recall (recall_sync) — recall the injected memory in-turn instead of next-turn prefetch (#5820)
- actionable error when local_embedded runtime is missing — tells the user which package to install (#7718)
- default retain_source to 'hermes' so every stored memory self-identifies its provenance
- offer a starter memory template during hermes memory setup, plus warn before overwriting an already-configured bank
- warn when a configured memory provider reports unavailable (#2765)
- deterministic 'recalled N memories' recall indicator — Hermes itself emits a status line when auto-recall injects memory
- 'saving to memory' retain indicator — emitted the moment a turn is dispatched to the writer

Authored by @benfrank241 (ben.bartholomew@vectorize.io).
Salvaged from PR #74379.
2026-08-13 23:15:25 +05:30
flyingdoubleg
e9a7c18890 fix(memory): honor disabled toolsets for provider tools 2026-07-24 13:00:53 +05:30
teknium1
c356752b6b fix(memory): drain queued writes on shutdown 2026-07-17 04:55:58 -07:00
Erosika
8d1c96fd2f fix(memory): align external prefetch guard with fail-open contracts 2026-07-16 12:48:48 -07:00
LeonSGP43
d77c455d7d fix(memory): fail fast on stuck external prefetch 2026-07-16 12:48:48 -07:00
Kshitij Kapoor
e0ed5dc9ed refactor: address Phase-2 review findings on /new boundary handoff
- Return the boundary snapshot from
  _launch_session_boundary_memory_flush as a local value instead of
  staging it on self._session_boundary_snapshot. The instance-attr
  handoff could leak (no memory manager configured) or mis-fire a
  stale snapshot on a later /new if an exception hit between staging
  and consumption. A local variable eliminates the class; the helper
  also returns None when no memory manager is configured so
  new_session takes the inline-switch path.
- Drop the now-dead session_id kwarg from commit_memory_session:
  after the redesign no production caller passes it (gateway, TUI,
  compression all use the default), and speculative params are
  rejected per AGENTS.md. The explicit-old-session need is served by
  cli.py's direct engine call + commit_session_boundary_async.
- Drop the dead providers snapshot in commit_session_boundary_async
  (only the emptiness check used it).
- Tests updated accordingly (dead-kwarg test removed, snapshot
  assertion now covered by return-value contract).

Phase-2 gates: 2a tests/cli 1048 passed + 6 memory files 137 passed;
2b programmatic live smoke 0.38ms non-blocking caller, end→switch→sync
ordering verified; 2c structured 4-angle review — no Criticals, these
warnings fixed.
2026-07-09 03:21:54 +05:30
Kshitij Kapoor
d8bc4f242f fix: serialize /new end→switch boundary on the memory manager worker
Deep review of the cherry-picked #16454 found the ad-hoc flush thread
raced new_session()'s inline on_session_switch(reset=True): memory
providers key off internal _session_id state (MemoryManager.on_session_end
takes no session id), so a late off-thread extraction ran against
post-rotation bindings — misattributing the old transcript to the new
session id, double-ingesting the old turn buffer (supermemory), or
double-committing (openviking already async-finalizes in
on_session_switch).

Redesign: new MemoryManager.commit_session_boundary_async queues
on_session_end + on_session_switch as ONE task on the manager's existing
single-worker background executor (the same worker sync_all already
uses). This preserves the strict end→switch ordering providers depend on,
serializes against per-turn syncs FIFO, keeps /new non-blocking, and
degrades to inline (pre-#16454 behavior) when the executor is
unavailable. No ad-hoc threads; no per-provider changes needed.

The context-engine on_session_end half stays synchronous in
_launch_session_boundary_memory_flush (cheap, must land before
reset_session_state rebinds the engine).

Exit durability: _run_cleanup calls the manager's existing
flush_pending(timeout=10) barrier before shutdown, so '/new then quit'
doesn't drop the queued extraction (shutdown_all's own drain is ~5s and
cancels queued tasks). Bounded well inside the 30s exit watchdog.

Tests: ordering invariant with slow (LLM-like) extraction, FIFO
serialization vs sync_all, switch-fires-even-if-end-raises, no-provider
no-op, CLI snapshot handoff + inline-switch fallback, sync engine
boundary, cleanup flush_pending.
2026-07-09 03:21:54 +05:30
Teknium
3f2a56d1a4 fix(cli): reliable interrupts, bounded exit, and exit feedback (#57000)
Three CLI reliability fixes:

1. Interrupt reliability: chat() only re-queued the user's interrupt
   message when the turn result carried interrupted=True. When the agent
   thread raced past its last interrupt check (or finished) before the
   interrupt landed, the message was silently dropped — and the stale
   _interrupt_requested flag left on the agent instantly aborted the
   NEXT turn. Un-acknowledged interrupt messages are now re-queued as
   the next turn and the stale flag is cleared (only when the agent
   thread actually exited). The clarify-race path also parks the message
   in _pending_input instead of dropping it.

2. Slow exit (5+ min): stdlib ThreadPoolExecutor workers are non-daemon
   and joined unconditionally by concurrent.futures' atexit hook — even
   after shutdown(wait=False). One wedged tool worker (abandoned after
   interrupt/timeout) held the process open forever. Promoted
   async_delegation's daemon executor to a shared tools/daemon_pool
   module and adopted it in tool_executor (concurrent tool batches),
   memory_manager (background sync), delegate_tool (child timeout wrapper
   + batch fan-out), and skills_hub (source fan-out). Added a 30s exit
   watchdog (HERMES_EXIT_WATCHDOG_S) armed at _run_cleanup start as a
   backstop for wedged cleanup steps.

3. Exit jank: after prompt_toolkit tears down the input/status bars the
   terminal sat silent for the whole cleanup window, looking hung. Print
   'Shutting down… (finalizing session)' immediately at exit start.

E2E: live PTY interrupt of a foreground 'sleep 120' terminal tool now
aborts in ~1s and the typed message runs as the next turn; wedged-worker
+ wedged-cleanup subprocess exits in 5.8s (watchdog) instead of hanging.
2026-07-02 04:20:43 -07:00
Bartok9
710cd48fb1 fix(agent): validate context/memory tool schemas before wrapping
Closes #47707

Context engines and memory providers expose tool schemas via
get_tool_schemas(). agent_init.py wrapped each as
{"type":"function","function":_schema} without validating that
_schema carries a top-level name. A provider returning an entry already
in OpenAI tool form ({"type":"function","function":{...}}) was then
double-wrapped into a tool whose function has no name. Strict providers
(e.g. DeepSeek) reject the entire request with HTTP 400
'tools[N].function: missing field name', so one malformed schema
silently disables the whole toolset and breaks every turn. The schema
was also never added to valid_tool_names, so even lenient providers
could not call it.

Add a shared normalize_tool_schema() helper that unwraps an
already-wrapped entry and returns None for anything lacking a resolvable
string name. Wire it into the agent_init context-engine loop and all
three memory_manager surfaces (inject_memory_provider_tools,
add_provider routing index, get_all_tool_schemas), so a single bad
plugin schema is skipped with a warning instead of poisoning the
request.

Verification: 209 targeted agent/memory tests pass (incl. 9 new).
New tests assert the unwrap + skip-nameless behavior and fail without
the fix.
2026-06-25 02:17:29 +05:30
Teknium
b1b20270c4 refactor(memory): move write-mirror gating behind MemoryManager interface
The success/staged gating and op-expansion for mirroring built-in memory
writes to external providers lived in a standalone agent/memory_write_bridge.py
helper called inline from two core call sites (tool_executor.py,
agent_runtime_helpers.py). That left the mirror decision-making in the agent
loop, outside the memory-provider interface.

Fold it into a new MemoryManager.notify_memory_tool_write() entry point: the
loop now hands over the raw tool result + args and a metadata callback, and the
manager decides whether/what to mirror. Both core call sites collapse to a
single call; the orphan module is removed. No MemoryProvider ABC change.

Tests rewritten as behavior tests against the manager method.
2026-06-22 07:00:42 -07:00
Gille
013f9c8750 fix(memory): log CLI shutdown hook failures
Makes the CLI memory-provider shutdown path observable: log when CLI
cleanup calls memory shutdown (with session id + message count), warn
instead of swallowing CLI memory-shutdown exceptions, warn on
on_session_end failures during agent shutdown, and raise the
MemoryManager provider-hook failure log from debug to warning with a
traceback.

Salvaged from PR #49287 (authored by Gille / @helix4u).
2026-06-19 16:59:43 -07:00
Teknium
c2c55c4443 fix(memory): strip skill scaffolding for all providers, not just openviking
Generalizes #32663 (@ehz0ah). The slash-skill scaffolding pollution
affected every auto-syncing memory provider — mem0, hindsight, retaindb,
byterover, honcho, supermemory all store/embed the raw user turn, so a
/skill invocation poisoned their stores with the full skill body, not just
openviking.

- Lift the contributor's parser into agent/skill_commands.py as the canonical
  extract_user_instruction_from_skill_message(), co-located with the message
  builders so the markers can't drift.
- Strip once in MemoryManager.{prefetch_all,queue_prefetch_all,sync_all} —
  fixes the whole provider fan-out, bare /skill turns are skipped entirely.
- OpenViking's _derive_openviking_user_text() now delegates to the shared
  helper as defense-in-depth (no duplicated marker literals).
- Marker-drift regression now asserts against the canonical skill_commands
  constants; add manager-level coverage proving every provider gets clean text.
2026-06-16 10:37:37 -07:00
helix4u
2d474e39c7 fix(acp): preserve memory provider tools 2026-06-13 04:51:44 -07:00
Teknium
aa6f2775fa fix(memory): run end-of-turn sync off the turn thread (#41945)
A misconfigured/slow external memory provider could hold the agent in
the 'running' state for minutes after the final response was delivered.
MemoryManager.sync_all / queue_prefetch_all looped provider.sync_turn /
queue_prefetch INLINE on the turn-completion path; a provider making a
blocking network/daemon call (a broken Hindsight daemon was observed
blocking ~298s before failing) blocked run_conversation from returning.
Because every interface (CLI, TUI, gateway) marks the agent 'running'
until run_conversation returns, the agent stayed busy for the full block
and any follow-up message triggered an aggressive interrupt that dropped
the message.

Dispatch provider sync/prefetch to a lazily-created single-worker
background executor. sync_all / queue_prefetch_all return immediately;
work completes (or fails, logged) in the background. A single worker
serializes writes so turn N lands before turn N+1. flush_pending()
provides a barrier for session boundaries and deterministic tests.
shutdown_all() drains the executor with a bounded timeout so a wedged
provider can never hang teardown.

Builtin-only / no-provider sessions spawn no executor (zero new threads
in the common case).
2026-06-08 02:18:59 -07:00
Teknium
fe8920db18 fix(memory): reject memory tools that shadow core tool names (#40902)
A memory provider tool whose name collides with a built-in core tool
(e.g. clarify, delegate_task) was skipped from agent.tools at init but
lingered in MemoryManager._tool_to_provider, where the has_tool dispatch
branch could route a call to a tool that was never registered (#40466).

Block the collision at registration instead of patching dispatch:
- MemoryManager.add_provider rejects any tool whose name is in
  _HERMES_CORE_TOOLS (warn + skip), so it never enters the routing table.
- get_all_tool_schemas applies the same filter, so the manager never
  advertises a schema it would refuse to route.

Built-ins always win, matching the invariant used by the TTS/browser/
search provider registries. Makes the dispatch-hijack structurally
impossible regardless of branch ordering.

Closes #40466.
2026-06-06 18:44:09 -07:00
Teknium
e1951ce704 fix(memory): only forward rewound kwarg when set
The on_session_switch fan-out passed rewound=rewound unconditionally,
injecting rewound=False into every provider's **kwargs on the common
/resume, /branch, /new, and compression paths. Providers that capture
extra kwargs into an 'extra' dict (and the exact-dict-equality tests
guarding them) broke. Forward rewound only when truthy; /undo sets it
explicitly, everyone else stays clean.
2026-06-01 01:22:38 -07:00
SaguaroDev
31cfa08c66 feat(memory): add rewound kwarg to on_session_switch hook 2026-06-01 01:22:38 -07:00
Dave Heritage
5a95fb2e14 feat: expose completed-turn message context to memory providers
Adds an optional `messages` keyword to the `MemoryProvider.sync_turn`
contract so external/community memory plugins can receive the OpenAI-style
conversation message list for the completed turn — including assistant tool
calls and tool result content — not just the final assistant text.

Dispatch uses signature inspection (`_provider_sync_accepts_messages`): only
providers that declare a `messages` parameter (or `**kwargs`) receive it; all
existing in-tree providers keep their legacy text-only signature and are
called unchanged. No structured-trace envelope is added to core — providers
reconstruct whatever they need from the standard message list.

Also documents Memori as a standalone community memory provider.

Salvaged from #28065 — rebased onto current main.

Co-authored-by: Dave Heritage <david@memorilabs.ai>
2026-05-29 02:16:43 +05:30
墨綠BG
50e93f23f2 🐛 fix(memory): require newline after context tag 2026-05-18 10:53:08 -07:00