Skip to content

fix(memory): serialize tool calls with store teardown - #1037

Draft
yunyv wants to merge 2 commits into
modelscope:mainfrom
yunyv:fix/memory-tool-lifecycle-20261030
Draft

yunyv wants to merge 2 commits into
modelscope:mainfrom
yunyv:fix/memory-tool-lifecycle-20261030

Conversation

@yunyv

@yunyv yunyv commented Oct 30, 2026

Copy link
Copy Markdown

Change Summary

Keep memory tools inside the orchestrator's existing store-ownership lifecycle. MemoryTool delegates to handle_tool_call, but that path previously bypassed the store lock and terminal-close check. A late tool could restart a closed backend and change MEMORY.md, or overlap another store operation and teardown.

  • Serialize tool dispatch on the existing per-store lock and reject calls on a retired orchestrator, including callers that waited for the lock.
  • Check backend startup state under the shutdown lock, so teardown also waits for a tool whose asynchronous backend start is still pending.
  • Preserve live tool arguments/results and exception propagation. Include the configured formatter's adjustments in the touched file.

Related issue number

No existing issue is being closed. This completes the memory-tool path under the terminal-close/store-serialization contract introduced in #943; it does not change #1026's facts cache handling.

Validation

  • Real SDK regression through MemoryTool, MemoryOrchestrator, and the file backend: baseline 7 failures / 2 passes; final 9 passes.
  • python -m pytest tests/memory -q: final 188 passed, 74 skipped, 3 failed. With the identical tests on untouched source: 181 passed, 74 skipped, 10 failed. The three common failures require an unavailable ModelScope remote fixture or API-key configuration; no live provider testing is claimed.
  • Independent lifecycle/control checks: 18 passed including the nine regressions.
  • Changed-file configured pre-commit hooks, dependency compatibility, compilation, and diff checks pass. Full pre-commit was attempted on baseline and final and retains existing repository lint/style failures; it is not fully green.
  • Full application and hosted CI tests were not run.

Checklist

  • The pull request title is a good summary of the changes
  • 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 (no public configuration change)

This branch has not been deployed

No deployments
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.

1 participant