Skip to content

feat: harden unified memory and unify provider-aware thinking controls - #943

Open
alcholiclg wants to merge 42 commits into
modelscope:mainfrom
alcholiclg:fix/runtime-robustness
Open

feat: harden unified memory and unify provider-aware thinking controls#943
alcholiclg wants to merge 42 commits into
modelscope:mainfrom
alcholiclg:fix/runtime-robustness

Conversation

@alcholiclg

@alcholiclg alcholiclg commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Change Summary

Unified memory reliability

  • Prevent interrupted turns from marking unrelated history as ingested, and track in-flight ownership so background memory writes are never silently skipped or denied a retry.
  • Share one memory instance per store instead of per model, allowing model switches to reuse the same embedded store without file-lock conflicts.
  • Reconfigure shared memory instances in place when settings change:
    • apply recall_top_k and ingest_interval immediately;
    • safely drain and rebuild the backend for store-affecting changes;
    • preserve the shared instance so existing agents keep a valid reference.
  • Make close() terminal: drain scheduled writes, release the backend safely, and prevent delayed tasks or later reads from reopening a retired store.
  • Serialize reads, searches, writes, flushes, and teardown with a store lock scoped by event loop and path, avoiding dead-loop locks and search/write races.
  • Forward search limits correctly to Mem0.
  • Add dates to recalled memories so conflicting facts can be resolved in favor of the newer entry.
  • Configure Mem0 to preserve the user’s language and script during extraction while allowing explicit custom instructions to override the default.

Provider-aware thinking controls

  • Introduce a canonical reasoning_effort setting with auto, off, low, medium, high, and max levels.
  • Detect the thinking dialect from the endpoint and protocol, then normalize and clamp the requested level for OpenAI, DashScope, ModelScope, DeepSeek, Zhipu, Moonshot, MiniMax, OpenRouter, and Anthropic-compatible APIs.
  • Preserve manually supplied wire parameters as an escape hatch and avoid invalid combinations such as reasoning_effort with thinking_budget.
  • Send both enable_thinking and reasoning_effort on DashScope because the switch and effort level control different behavior; translate max to DashScope’s xhigh.
  • Apply lowering exactly once per request across the legacy OpenAI client, provider-router OpenAI transport, and Anthropic Messages transport, before signature filtering can discard the canonical setting.
  • Retry once with thinking explicitly disabled when a model rejects thinking parameters, then memoize the endpoint/model result to prevent repeated failed requests.
  • Handle models where reasoning is mandatory by removing the disabling parameters, retrying once, and remembering the constraint without suppressing later positive effort levels.
  • Keep unrelated 400 responses unchanged.
  • Read both reasoning_content and OpenRouter’s reasoning response field so proxied reasoning remains visible.
  • Provide optional MSA_DEBUG_THINKING diagnostics for inspecting the lowered request.

Shell command permissions

  • Confirm network-egress commands (curl, wget, ssh, scp, rsync, nc, netcat) instead of refusing them: as blacklist entries no mode or user answer could override them, and blocking by command name never stopped npm, pip, or git from reaching the network anyway — they are ask rules now, enforced on the ordinary permission path where ask_rules had been inert (allow_network: true opts out).
  • Stop a bare * in dangerous_removal_paths from being applied as an fnmatch glob, which marked every path dangerous and had the non-bypassable safety layer refuse every rm, rm build/out.txt included; rm -rf /, rm *, and rm ~ stay refused.
  • Remember the command that was approved rather than the bare tool name when the caller supplies no pattern, so one allow_always on ls -la no longer releases the whole shell, and match a bare <cmd> against its own <cmd> * pattern.

Multimodal image input

  • Accept image attachments end to end — composer upload → message → session log → provider wire → history replay — widening the composer-to-SDK boundary, which previously carried text only. Multiple images are labelled Image N: <filename> so later turns can reference them, and are rescaled or transcoded to fit each provider's limits before sending.
  • Carry images as references (workspace path + media type) rather than bytes, expanded into provider-native blocks inside each transport. Session logs stay free of base64, a mid-session model switch re-exposes images already in history, and encoding is recomputed per provider instead of frozen at write time.
  • Add a per-model "image understanding" setting read as a tri-state — explicit choice, else provider capability, else an observed refusal — and surface it in the composer before sending when it is off.
  • Retry once with images replaced by text when an endpoint rejects an image-carrying request, remembering the model only when that retry succeeds, so an unrelated 400 neither disables images on a capable model nor masks the real error. Write the result back to the model setting.
  • Return a short status plus a structured image reference from file_system---read_file instead of base64 in the text channel — inline in tool_result for Anthropic, hoisted into a following user message for the OpenAI family, whose Chat Completions schema restricts tool message content to text parts. Charge image blocks a flat per-image cost in the context estimator, which previously measured base64 length and could let a single image exceed the whole context budget.
  • Route multimodal content through a shared flatten_message_text in memory recall, compaction, hooks, session naming and history rendering so it cannot silently degrade, and add an optional image_reader tool that describes an image through a separately configured vision model.

Related issue number

Checklist

  • The pull request title is a good summary of the changes - it will be used in the changelog
  • Unit tests for the changes exist
  • Run pre-commit install and pre-commit run --all-files before git commit, and passed lint check.
  • Documentation reflects the changes where applicable

The orchestrator now owns the write discipline around a backend:
- schedule_add() runs the extraction-LLM + embedding cost (seconds) in a
  background task; flush_pending() is the teardown barrier so the last
  write is never dropped, and an inline fallback keeps writes when no
  loop is running.
- retrieval/ingestion/flush serialize on one per-store asyncio lock
  (embedded qdrant underneath is lock-free single-client code).
- a content-hash delta ledger (<base_dir>/ingest_state.json) makes each
  ingest send only messages the store has not seen; hashes are recorded
  only after a confirmed write, so a failed ingest retries naturally.
- ingest_status reports the last outcome (state/count/error/pending) so
  a UI can show memory working instead of silence.

Mem0Backend: per-turn retrieval cache (rounds 2..N of a tool-calling
turn reuse round 1's search instead of paying an embedding round-trip
each), on_messages returns the event count and propagates failures --
the orchestrator is the swallow-and-report layer now and needs the
exception to keep failed messages un-marked for retry.
…-side close

- add_memory(add_after_step) now fires only when a round closes the turn
  (assistant reply with no tool calls) and dispatches through the
  backend's schedule_add when available: tool rounds are intermediate
  state, and ingesting every round cost O(rounds x history) extraction
  calls where the closing ingest covers the whole turn.
- an interrupted round advances the ingest ledger WITHOUT ingesting
  (mark_ingested): a half-finished answer is not durable conversational
  truth and must not be swept into the next turn's delta.
- cleanup_tools drains scheduled ingestion (flush only -- memory
  instances are shared across agents of one store, so closing here would
  yank the store from a sibling agent); the new
  SharedMemoryManager.close_matching(base_dir) is the owner-of-last-
  resort that actually closes instances and releases the embedded
  store's exclusive file lock.
The number of recalled memories injected per turn was hardcoded twice
(search default 20, then a [:10] formatting slice). MemoryConfig gains
recall_top_k (default 10, read from the unified_memory node) and the
mem0 adapter threads it through search and formatting — consumers can
now size recall to their context budget.
…E) instead of scattered config fields, gated by personalization.enabled.

When those files change mid-conversation the next user turn carries a durable <system-reminder> naming them, so the model can tell a changed file from its own faulty memory.
…end's MEMORY.md snapshot in step with edits made outside the agent.

Also translates the memory tool descriptions and prompt headings to English.
# Conflicts:
#	.gitignore
#	ms_agent/memory/unified/backends/mem0_adapter.py
#	ms_agent/memory/unified/orchestrator.py
#	setup.py
…ts last entry deleted, instead of leaving the previous round's block in place.
- an interrupt marks only its own round, and never messages a scheduled
  ingest still owns (both lost the write silently)
- one shared instance per store, not per model, reconfigured in place
- close() is terminal: a straggler can no longer reopen a released store
- the store lock is per (loop, path), and search() takes it too
- search() honours its limit; injected memories carry their date
- memories are written in the language the user used
# Conflicts:
#	ms_agent/memory/unified/backends/mem0_adapter.py
…ters

Thinking support is per-model with no naming rule, and an unsupported model may
reject the whole request (DashScope returns 400) instead of ignoring the flag.
So we ask, and on a refusal retry once with it off, remembering the model.
…orwards, and read OpenRouter's reasoning field
…hat arrive mid-stream

- images go out only when the model's own switch says so; a provider's declared
  vision capability no longer implies it
- a 400 delivered on the first streamed chunk is repaired like an eager one
- thinking refusals are repaired on the Anthropic and Responses paths too
- a tool call the model is still writing is reported instead of nothing at all
- an unreadable managed MCP config is logged instead of silently yielding none
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants