From eba1de2aa9f73745afd3b15f44b1c6df1843da31 Mon Sep 17 00:00:00 2001 From: Erick Date: Tue, 26 May 2026 06:32:23 -0700 Subject: [PATCH 1/6] fix(migrations): remove duplicate ALTER TABLE stmts from 008 now in 001 All columns added by 008-feedback-experience-metadata were backported into 001-initial.sql; the ALTER TABLE statements now fail on fresh DBs. Keeps only the idempotent CREATE INDEX lines. Also fix scripts/replay.py bridge_client import path (adapters/ is under apps/memos-local-plugin/, not the repo root) and add scripts/export-state-sessions.py for exporting state.db sessions to JSON files compatible with replay.py. Co-Authored-By: Claude Sonnet 4.6 --- .../008-feedback-experience-metadata.sql | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/apps/memos-local-plugin/core/storage/migrations/008-feedback-experience-metadata.sql b/apps/memos-local-plugin/core/storage/migrations/008-feedback-experience-metadata.sql index a25ef2ae7..bdf5f385a 100644 --- a/apps/memos-local-plugin/core/storage/migrations/008-feedback-experience-metadata.sql +++ b/apps/memos-local-plugin/core/storage/migrations/008-feedback-experience-metadata.sql @@ -1,20 +1,5 @@ -- Add explicit metadata for feedback-derived experiences. --- Existing policies are treated as success-backed procedural experience so --- skill crystallization keeps its legacy behavior for old rows. -ALTER TABLE policies ADD COLUMN experience_type TEXT NOT NULL DEFAULT 'success_pattern' - CHECK (experience_type IN ('success_pattern','repair_validated','failure_avoidance','repair_instruction','preference','verifier_feedback','procedural')); -ALTER TABLE policies ADD COLUMN evidence_polarity TEXT NOT NULL DEFAULT 'positive' - CHECK (evidence_polarity IN ('positive','negative','neutral','mixed')); -ALTER TABLE policies ADD COLUMN salience REAL NOT NULL DEFAULT 0; -ALTER TABLE policies ADD COLUMN confidence REAL NOT NULL DEFAULT 0.5; -ALTER TABLE policies ADD COLUMN source_feedback_ids_json TEXT NOT NULL DEFAULT '[]' - CHECK (json_valid(source_feedback_ids_json)); -ALTER TABLE policies ADD COLUMN source_trace_ids_json TEXT NOT NULL DEFAULT '[]' - CHECK (json_valid(source_trace_ids_json)); -ALTER TABLE policies ADD COLUMN verifier_meta_json TEXT NOT NULL DEFAULT 'null' - CHECK (json_valid(verifier_meta_json)); -ALTER TABLE policies ADD COLUMN skill_eligible INTEGER NOT NULL DEFAULT 1 - CHECK (skill_eligible IN (0,1)); +-- All ALTER TABLE columns backported into 001-initial.sql; only indexes remain here. CREATE INDEX IF NOT EXISTS idx_policies_experience ON policies(experience_type, evidence_polarity, updated_at DESC); From 123d860b88b14b57bb7111886ee94998707cba54 Mon Sep 17 00:00:00 2001 From: Erick Date: Sat, 30 May 2026 22:02:07 -0700 Subject: [PATCH 2/6] fix(daemon): rebuild when any .ts source is newer than dist/bridge.cjs _rebuild_if_stale() only checked bridge.cts mtime, so changes to core/**/*.ts (e.g. memory-core.ts) didn't trigger an auto-rebuild on gateway restart. Walk all .ts files under plugin_root and compare the newest mtime against the compiled binary instead. --- .../hermes/memos_provider/daemon_manager.py | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/apps/memos-local-plugin/adapters/hermes/memos_provider/daemon_manager.py b/apps/memos-local-plugin/adapters/hermes/memos_provider/daemon_manager.py index 19c8a7be0..fb595c0e3 100644 --- a/apps/memos-local-plugin/adapters/hermes/memos_provider/daemon_manager.py +++ b/apps/memos-local-plugin/adapters/hermes/memos_provider/daemon_manager.py @@ -147,6 +147,55 @@ def _bridge_command(*, daemon: bool) -> list[str]: return [node, "--import", "tsx", *bridge_args] +def _rebuild_if_stale() -> bool: + """Run `npm run build` if any TypeScript source is newer than dist/bridge.cjs. + + Returns True if the binary is current or the build succeeded. + Returns False if the build failed — caller should continue with + the existing binary rather than blocking gateway startup. + """ + plugin_root = _plugin_root() + compiled = plugin_root / "dist" / "bridge.cjs" + + if compiled.exists(): + compiled_mtime = compiled.stat().st_mtime + newest_source = max( + (p.stat().st_mtime for p in plugin_root.rglob("*.ts")), + default=0.0, + ) + if newest_source <= compiled_mtime: + return True + + logger.info("MemOS: TypeScript source newer than dist/bridge.cjs — rebuilding...") + npm = shutil.which("npm") + if not npm: + logger.warning("MemOS: npm not found on PATH; skipping bridge rebuild") + return False + try: + result = subprocess.run( + [npm, "run", "build"], + cwd=str(plugin_root), + check=True, + timeout=120, + capture_output=True, + text=True, + ) + logger.info("MemOS: bridge daemon rebuilt successfully") + if result.stderr: + logger.debug("MemOS: build stderr: %s", result.stderr.strip()) + return True + except subprocess.CalledProcessError as e: + logger.error("MemOS: bridge rebuild failed:\n%s", e.stderr) + return False + except subprocess.TimeoutExpired: + logger.error("MemOS: bridge rebuild timed out after 120s") + return False + except Exception as e: + logger.error("MemOS: bridge rebuild error: %s", e) + return False + + + def ensure_bridge_running(*, probe_only: bool = False) -> bool: """Return True when the bridge is (or can be) operational. From 38fbcfc8ee8739b95bc5da253fea264d584e00ae Mon Sep 17 00:00:00 2001 From: Memtensor-AI Date: Sun, 14 Jun 2026 17:24:41 +0800 Subject: [PATCH 3/6] Fix #1540: fix: (#1824) docs(memos-local-plugin): clarify install path and stale dir names (#1540) The README's 'Quick start' section told users to use install.sh instead of npm install, but the warning was buried and users still tried 'npm install -g @memtensor/memos-local-plugin' first. The reporter in #1540 encountered this on a Hermes deployment. This change: - Promotes the 'do not run npm install -g' notice to a prominent IMPORTANT callout explaining why global install is wrong (no agent-home deploy, no config.yaml, no bridge/viewer) and that the tarball intentionally ships built artifacts only. - Adds a Troubleshooting subsection covering the two specific symptoms in the bug report: the 'package not found' misread, and the stale web/ and site/ directory names (web/ is now viewer/, site/ was removed by commit 26e7e3db). - Mentions install.ps1 for Windows alongside install.sh. - CHANGELOG: record the docs fix and reference #1540. Documentation-only change; no code or runtime behavior touched. Co-authored-by: MemOS AutoDev Co-authored-by: Matthew From 576b408154523eba2765f9ee7226f12cd4a6141f Mon Sep 17 00:00:00 2001 From: Memtensor-AI Date: Sun, 14 Jun 2026 17:54:02 +0800 Subject: [PATCH 4/6] Fix #1888: [Bug] test_system_parser.py: SystemParser.__init__() got an unexpected keyword a (#1889) fix: remove invalid chunker parameter from SystemParser test instantiation - SystemParser.__init__() signature changed to (embedder, llm=None) - Test was still passing chunker=None causing TypeError - Fixes all 5 failing tests in test_system_parser.py Fixes #1888 Co-authored-by: MemOS AutoDev Co-authored-by: Matthew From 968930faf0210a50e91a0de42677777ea77f0e40 Mon Sep 17 00:00:00 2001 From: Memtensor-AI Date: Sun, 14 Jun 2026 22:29:42 +0800 Subject: [PATCH 5/6] Fix #1525: [Bug] clean_json_response crashes with cryptic AttributeError when given None (#1884) * test: add comprehensive tests for clean_json_response (issue #1525) - Add test suite in tests/mem_os/test_format_utils.py - Cover None input ValueError with diagnostic message - Cover markdown removal, whitespace stripping, edge cases - Verify fix for AttributeError when LLM returns None * style: format clean_json_response tests --------- Co-authored-by: MemOS AutoDev Co-authored-by: Matthew --- tests/mem_os/test_format_utils.py | 75 +++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100644 tests/mem_os/test_format_utils.py diff --git a/tests/mem_os/test_format_utils.py b/tests/mem_os/test_format_utils.py new file mode 100644 index 000000000..b97178784 --- /dev/null +++ b/tests/mem_os/test_format_utils.py @@ -0,0 +1,75 @@ +""" +Test suite for src/memos/mem_os/utils/format_utils.py + +Focus: clean_json_response function defensive behavior +Related issue: #1525 +""" + +import pytest + +from memos.mem_os.utils.format_utils import clean_json_response + + +class TestCleanJsonResponse: + """Test clean_json_response function with various inputs.""" + + def test_clean_json_response_with_none_raises_value_error(self): + """Test that passing None raises ValueError with diagnostic message.""" + with pytest.raises(ValueError) as exc_info: + clean_json_response(None) + + error_message = str(exc_info.value) + assert "clean_json_response received None" in error_message + assert "upstream LLM call" in error_message + assert "timed_with_status" in error_message or "generate()" in error_message + + def test_clean_json_response_removes_json_code_block(self): + """Test removal of ```json markers.""" + input_str = '```json\n{"key": "value"}\n```' + expected = '{"key": "value"}' + assert clean_json_response(input_str) == expected + + def test_clean_json_response_removes_plain_code_block(self): + """Test removal of ``` markers without json keyword.""" + input_str = '```\n{"key": "value"}\n```' + expected = '{"key": "value"}' + assert clean_json_response(input_str) == expected + + def test_clean_json_response_strips_whitespace(self): + """Test that leading/trailing whitespace is stripped.""" + input_str = ' \n {"key": "value"} \n ' + expected = '{"key": "value"}' + assert clean_json_response(input_str) == expected + + def test_clean_json_response_handles_plain_json(self): + """Test that plain JSON without markdown is unchanged (except strip).""" + input_str = '{"key": "value"}' + expected = '{"key": "value"}' + assert clean_json_response(input_str) == expected + + def test_clean_json_response_handles_empty_string(self): + """Test that empty string is handled correctly.""" + assert clean_json_response("") == "" + + def test_clean_json_response_with_complex_json(self): + """Test with realistic LLM response containing nested JSON.""" + input_str = """```json +{ + "queries": [ + {"query": "test", "weight": 1.0}, + {"query": "example", "weight": 0.5} + ] +} +```""" + result = clean_json_response(input_str) + assert "```json" not in result + assert "```" not in result + assert '"queries"' in result + assert result.strip() == result # No leading/trailing whitespace + + def test_clean_json_response_preserves_internal_backticks(self): + """Test that backticks inside JSON content are preserved.""" + input_str = '```json\n{"code": "`example`"}\n```' + result = clean_json_response(input_str) + assert "`example`" in result + assert result.count("`") == 2 # Only internal backticks remain From 8bfef2597f6913541e72eccae42998ca2e254f0f Mon Sep 17 00:00:00 2001 From: Memtensor-AI Date: Mon, 15 Jun 2026 00:10:23 +0800 Subject: [PATCH 6/6] =?UTF-8?q?Fix=20#1901:=20share=5Fcube=5Fwith=5Fuser?= =?UTF-8?q?=20passes=20swapped=20args=20to=20=5Fvalidate=5Fcube=5Faccess?= =?UTF-8?q?=20=E2=80=94=20fails=20for=20ev=20(#1903)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fix: validate current user not target in share_cube_with_user (#1901) share_cube_with_user(cube_id, target_user_id) called _validate_cube_access(cube_id, target_user_id), but the validator signature is (user_id, cube_id). The cube_id therefore landed in the user_id slot and _validate_user_exists raised "User '' does not exist or is inactive" for every well-formed call, making the API unusable. The in-code comment "Validate current user has access to this cube" already documented the correct intent: the sharing user (self.user_id) must have access to the cube being shared, not the target. Switch the call to self._validate_cube_access(self.user_id, cube_id). The target user's existence is independently checked on the next line via validate_user(target_user_id), so that path is unchanged. Add regression tests in tests/mem_os/test_memos_core.py that pin down: - validate_user_cube_access is consulted with (self.user_id, cube_id), - add_user_to_cube is called with (target_user_id, cube_id) on success, - a missing target raises "Target user '' does not exist". Closes #1901 Co-authored-by: MemOS AutoDev Bot Co-authored-by: Matthew --- src/memos/mem_os/core.py | 2 +- tests/mem_os/test_memos_core.py | 143 ++++++++++++++++++++++++++++++++ 2 files changed, 144 insertions(+), 1 deletion(-) diff --git a/src/memos/mem_os/core.py b/src/memos/mem_os/core.py index 54f8f01e0..3ede965d3 100644 --- a/src/memos/mem_os/core.py +++ b/src/memos/mem_os/core.py @@ -1170,7 +1170,7 @@ def share_cube_with_user(self, cube_id: str, target_user_id: str) -> bool: bool: True if successful, False otherwise. """ # Validate current user has access to this cube - self._validate_cube_access(cube_id, target_user_id) + self._validate_cube_access(self.user_id, cube_id) # Validate target user exists if not self.user_manager.validate_user(target_user_id): diff --git a/tests/mem_os/test_memos_core.py b/tests/mem_os/test_memos_core.py index 6d2408d05..b57b0b254 100644 --- a/tests/mem_os/test_memos_core.py +++ b/tests/mem_os/test_memos_core.py @@ -795,3 +795,146 @@ def test_search_nonexistent_cube( assert result["text_mem"] == [] assert result["act_mem"] == [] assert result["para_mem"] == [] + + +class TestShareCubeWithUser: + """Regression tests for share_cube_with_user (issue #1901). + + The original implementation called ``_validate_cube_access(cube_id, + target_user_id)``, which both (a) swapped the positional arguments and + (b) validated the wrong user. Every well-formed call therefore failed + with ``ValueError: User '' does not exist or is inactive`` even + though the calling user owned the cube. These tests pin down the correct + semantics: validate the *current* user against the cube being shared, + then delegate the share to ``user_manager.add_user_to_cube``. + """ + + def _build_mos( + self, + mock_llm_factory, + mock_reader_factory, + mock_user_manager_class, + mock_config, + mock_llm, + mock_mem_reader, + mock_user_manager, + ): + mock_llm_factory.from_config.return_value = mock_llm + mock_reader_factory.from_config.return_value = mock_mem_reader + mock_user_manager_class.return_value = mock_user_manager + return MOSCore(MOSConfig(**mock_config)) + + @patch("memos.mem_os.core.UserManager") + @patch("memos.mem_os.core.MemReaderFactory") + @patch("memos.mem_os.core.LLMFactory") + def test_share_cube_validates_current_user_not_target( + self, + mock_llm_factory, + mock_reader_factory, + mock_user_manager_class, + mock_config, + mock_llm, + mock_mem_reader, + mock_user_manager, + ): + """Cube access must be validated against the *current* user. + + Regression for #1901: previously the cube_id was passed where the + user_id was expected, causing ``_validate_user_exists`` to reject + every call because the cube UUID is obviously not a registered user. + """ + mock_user_manager.validate_user.return_value = True + mock_user_manager.validate_user_cube_access.return_value = True + mock_user_manager.add_user_to_cube.return_value = True + + mos = self._build_mos( + mock_llm_factory, + mock_reader_factory, + mock_user_manager_class, + mock_config, + mock_llm, + mock_mem_reader, + mock_user_manager, + ) + + cube_id = "cube-uuid-1234" + target_user_id = "target_user" + + result = mos.share_cube_with_user(cube_id=cube_id, target_user_id=target_user_id) + + assert result is True + # The cube-access check must be made against the *current* user, + # not the cube_id and not the target user. + mock_user_manager.validate_user_cube_access.assert_called_once_with(mos.user_id, cube_id) + # And the actual sharing must add the *target* user to the cube. + mock_user_manager.add_user_to_cube.assert_called_once_with(target_user_id, cube_id) + + @patch("memos.mem_os.core.UserManager") + @patch("memos.mem_os.core.MemReaderFactory") + @patch("memos.mem_os.core.LLMFactory") + def test_share_cube_raises_when_current_user_lacks_access( + self, + mock_llm_factory, + mock_reader_factory, + mock_user_manager_class, + mock_config, + mock_llm, + mock_mem_reader, + mock_user_manager, + ): + """If the current user doesn't have access to the cube, refuse to share. + + The error message must reference the current user, not the cube_id + (which was the misleading symptom in #1901). + """ + mock_user_manager.validate_user.return_value = True + mock_user_manager.validate_user_cube_access.return_value = False + + mos = self._build_mos( + mock_llm_factory, + mock_reader_factory, + mock_user_manager_class, + mock_config, + mock_llm, + mock_mem_reader, + mock_user_manager, + ) + + with pytest.raises(ValueError, match="test_user"): + mos.share_cube_with_user(cube_id="cube-uuid-1234", target_user_id="target_user") + + mock_user_manager.add_user_to_cube.assert_not_called() + + @patch("memos.mem_os.core.UserManager") + @patch("memos.mem_os.core.MemReaderFactory") + @patch("memos.mem_os.core.LLMFactory") + def test_share_cube_raises_when_target_user_missing( + self, + mock_llm_factory, + mock_reader_factory, + mock_user_manager_class, + mock_config, + mock_llm, + mock_mem_reader, + mock_user_manager, + ): + """Target user must exist; ``validate_user`` is consulted independently.""" + # validate_user is used twice: once during MOSCore.__init__ for + # ``self.user_id`` (must succeed) and once for the target user (fail). + mock_user_manager.validate_user.side_effect = lambda uid: uid == "test_user" + mock_user_manager.validate_user_cube_access.return_value = True + + mos = self._build_mos( + mock_llm_factory, + mock_reader_factory, + mock_user_manager_class, + mock_config, + mock_llm, + mock_mem_reader, + mock_user_manager, + ) + + with pytest.raises(ValueError, match="Target user 'missing_user'"): + mos.share_cube_with_user(cube_id="cube-uuid-1234", target_user_id="missing_user") + + mock_user_manager.add_user_to_cube.assert_not_called()