test: memory status B1 regression + sync_helpers coverage (12 new tests)

src/export_import.py

@@ -271,9 +271,12 @@ def export_cmd(path: str, no_secrets: bool, yes: bool):
     use_encryption = not no_secrets and _check_pyrage()
 
     if not no_secrets and not _check_pyrage():
-        warning("pyrage not installed — exporting without secret encryption.")
-        warning("Install with: pip install pyrage")
-        use_encryption = False
+        error(
+            "pyrage is not installed — secrets cannot be encrypted.\n"
+            "  Install:          pip install pyrage\n"
+            "  Export without secrets: apc export --no-secrets"
+        )
+        raise SystemExit(1)
 
     if use_encryption:
         public_key, _priv = _load_or_create_identity()

src/status.py

@@ -46,9 +46,9 @@ def _tool_sync_status(name: str) -> str:
         if fp := info_dict.get("link_path"):
             recorded_paths.append(fp)
 
-    for info_dict in manifest._data.get("memory", {}).values():
-        if fp := info_dict.get("file_path"):
-            recorded_paths.append(fp)
+    # memory is a flat dict {file_path, checksum, ...}, not a dict-of-dicts.
+    if fp := manifest._data.get("memory", {}).get("file_path"):
+        recorded_paths.append(fp)
 
     # If nothing was recorded (e.g. only MCP servers were synced), trust the timestamp
     if not recorded_paths:

src/sync_helpers.py

@@ -93,21 +93,25 @@ def sync_skills(tool_list: List[str]) -> Tuple[int, int]:
             applier = get_applier(tool_name)
             manifest = applier.get_manifest()
 
+            # Per-tool counts (reset each iteration)
+            tool_copy = 0
+            tool_link = 0
+
             # Copy collected skills
             if collected_skills:
-                c = applier.apply_skills(collected_skills, manifest)
-                total_copy += c
+                tool_copy = applier.apply_skills(collected_skills, manifest)
+                total_copy += tool_copy
 
             # Link installed skills
             if installed_skills:
-                lk = applier.link_skills(installed_skills, skills_dir, manifest)
-                total_link += lk
+                tool_link = applier.link_skills(installed_skills, skills_dir, manifest)
+                total_link += tool_link
 
             # Prune orphaned skills (keep MCP names empty — not our concern)
             applier.prune(all_skill_names, [], manifest)
             manifest.save()
 
-            success(f"{tool_name}: {total_copy} copied, {total_link} linked")
+            success(f"{tool_name}: {tool_copy} copied, {tool_link} linked")
         except Exception as e:
             error(f"Failed to sync skills to {tool_name}: {e}")
 

tests/test_export_import.py

@@ -309,6 +309,43 @@ def setUp(self):
         self.tmpdir = tempfile.mkdtemp()
         self.export_dir = Path(self.tmpdir) / "test-export"
 
+    @patch("export_import._check_pyrage", return_value=False)
+    def test_export_fails_loudly_without_pyrage(self, _mock_pyrage):
+        """Without pyrage and --no-secrets unset, export must abort — no silent plaintext."""
+        from click.testing import CliRunner
+
+        runner = CliRunner()
+        result = runner.invoke(export_cmd, [str(self.export_dir), "--yes"])
+
+        # Must NOT succeed — secrets would be plaintext
+        self.assertNotEqual(result.exit_code, 0)
+        self.assertFalse(self.export_dir.exists(), "Export dir should not be created on abort")
+
+    @patch("export_import._check_pyrage", return_value=False)
+    @patch("export_import.get_skills_dir")
+    @patch("export_import.get_config_dir")
+    @patch("export_import.load_mcp_servers", return_value=[])
+    @patch(
+        "export_import.load_local_bundle",
+        return_value={"skills": [], "mcp_servers": [], "memory": []},
+    )
+    def test_export_no_secrets_succeeds_without_pyrage(
+        self, mock_bundle, mock_mcp, mock_config, mock_skills, _mock_pyrage
+    ):
+        """--no-secrets works fine even when pyrage is unavailable."""
+        config_dir = Path(self.tmpdir) / "config"
+        config_dir.mkdir()
+        mock_config.return_value = config_dir
+        mock_skills.return_value = config_dir / "skills"
+
+        from click.testing import CliRunner
+
+        runner = CliRunner()
+        result = runner.invoke(export_cmd, [str(self.export_dir), "--yes", "--no-secrets"])
+
+        self.assertEqual(result.exit_code, 0, result.output)
+        self.assertTrue((self.export_dir / "apc-export.json").exists())
+
     @patch("export_import._check_pyrage", return_value=False)
     @patch("export_import.get_skills_dir")
     @patch("export_import.get_config_dir")
@@ -328,7 +365,7 @@ def test_export_creates_structure(
         from click.testing import CliRunner
 
         runner = CliRunner()
-        result = runner.invoke(export_cmd, [str(self.export_dir), "--yes"])
+        result = runner.invoke(export_cmd, [str(self.export_dir), "--yes", "--no-secrets"])
 
         self.assertEqual(result.exit_code, 0, result.output)
         self.assertTrue((self.export_dir / "apc-export.json").exists())
@@ -363,7 +400,7 @@ def test_export_writes_cache_data(
         from click.testing import CliRunner
 
         runner = CliRunner()
-        result = runner.invoke(export_cmd, [str(self.export_dir), "--yes"])
+        result = runner.invoke(export_cmd, [str(self.export_dir), "--yes", "--no-secrets"])
 
         self.assertEqual(result.exit_code, 0, result.output)
 

tests/test_manifest.py

@@ -251,3 +251,61 @@ def test_partial_files_missing_is_out_of_sync(self):
             side_effect=lambda name: ToolManifest(name, path=self.manifest_path),
         ):
             assert _tool_sync_status("cursor") == "out of sync"
+
+    # -- memory path tests (B1 regression) --------------------------------
+
+    def test_synced_when_memory_file_present(self):
+        """Memory file recorded in manifest and present on disk → synced.
+
+        Regression: the old code iterated _data["memory"].values() treating
+        the flat memory dict as a dict-of-dicts, causing AttributeError on
+        str.get().  This test would crash before the fix.
+        """
+        from status import _tool_sync_status
+
+        memory_file = self.tmpdir / "CLAUDE.md"
+        memory_file.write_text("# My context")
+
+        m = self._make_manifest()
+        m.record_memory(file_path=str(memory_file), content="# My context", entry_ids=[])
+        m.save()
+
+        with unittest.mock.patch(
+            "status.ToolManifest",
+            side_effect=lambda name: ToolManifest(name, path=self.manifest_path),
+        ):
+            assert _tool_sync_status("cursor") == "synced"
+
+    def test_out_of_sync_when_memory_file_deleted(self):
+        """Memory file recorded but deleted from disk → out of sync."""
+        from status import _tool_sync_status
+
+        memory_file = self.tmpdir / "CLAUDE.md"
+        memory_file.write_text("# My context")
+
+        m = self._make_manifest()
+        m.record_memory(file_path=str(memory_file), content="# My context", entry_ids=[])
+        m.save()
+
+        memory_file.unlink()  # simulate deletion
+
+        with unittest.mock.patch(
+            "status.ToolManifest",
+            side_effect=lambda name: ToolManifest(name, path=self.manifest_path),
+        ):
+            assert _tool_sync_status("cursor") == "out of sync"
+
+    def test_synced_when_memory_not_yet_recorded(self):
+        """Manifest exists (MCP synced) but no memory recorded → synced (not out of sync)."""
+        from status import _tool_sync_status
+
+        m = self._make_manifest()
+        m.record_mcp_server("filesystem")
+        m.save()
+        # memory dict is empty {} at this point
+
+        with unittest.mock.patch(
+            "status.ToolManifest",
+            side_effect=lambda name: ToolManifest(name, path=self.manifest_path),
+        ):
+            assert _tool_sync_status("cursor") == "synced"

tests/test_sync_helpers.py

@@ -0,0 +1,194 @@
+"""Unit tests for sync_helpers — sync_all, sync_skills, resolve_target_tools."""
+
+import tempfile
+import unittest
+import unittest.mock
+from pathlib import Path
+from unittest.mock import MagicMock, patch
+
+from appliers.manifest import ToolManifest
+
+
+def _make_manifest(tmpdir: Path, tool: str = "cursor") -> ToolManifest:
+    return ToolManifest(tool, path=tmpdir / f"{tool}.json")
+
+
+def _mock_applier(tmpdir: Path, tool: str = "cursor"):
+    """Return a MagicMock that satisfies the applier interface."""
+    applier = MagicMock()
+    applier.get_manifest.return_value = _make_manifest(tmpdir, tool)
+    applier.apply_skills.return_value = 3
+    applier.link_skills.return_value = 1
+    applier.apply_mcp_servers.return_value = 2
+    applier.apply_memory_via_llm.return_value = 1
+    applier.prune.return_value = None
+    return applier
+
+
+class TestResolveTargetTools(unittest.TestCase):
+    """resolve_target_tools: --tools flag / --all / interactive."""
+
+    def test_tools_flag_parsed(self):
+        from sync_helpers import resolve_target_tools
+
+        result = resolve_target_tools("cursor,claude-code", apply_all=False)
+        self.assertEqual(result, ["cursor", "claude-code"])
+
+    def test_tools_flag_strips_whitespace(self):
+        from sync_helpers import resolve_target_tools
+
+        result = resolve_target_tools("  cursor , claude-code  ", apply_all=False)
+        self.assertEqual(result, ["cursor", "claude-code"])
+
+    def test_tools_flag_empty_returns_empty(self):
+        from sync_helpers import resolve_target_tools
+
+        result = resolve_target_tools("", apply_all=False)
+        self.assertEqual(result, [])
+
+    def test_apply_all_uses_detected_tools(self):
+        from sync_helpers import resolve_target_tools
+
+        with patch("sync_helpers.detect_installed_tools", return_value=["cursor", "claude-code"]):
+            result = resolve_target_tools(None, apply_all=True)
+
+        self.assertEqual(result, ["cursor", "claude-code"])
+
+    def test_apply_all_no_tools_returns_empty(self):
+        from sync_helpers import resolve_target_tools
+
+        with patch("sync_helpers.detect_installed_tools", return_value=[]):
+            result = resolve_target_tools(None, apply_all=True)
+
+        self.assertEqual(result, [])
+
+
+class TestSyncAll(unittest.TestCase):
+    """sync_all: happy path, partial failure, all-fail, no-memory flag."""
+
+    def setUp(self):
+        self.tmpdir = Path(tempfile.mkdtemp())
+        self.bundle = {
+            "skills": [{"name": "my-skill", "body": "# Instructions"}],
+            "mcp_servers": [{"name": "filesystem", "transport": "stdio", "command": "npx"}],
+            "memory": [{"id": "abc", "source_tool": "openclaw", "content": "# Context"}],
+        }
+
+    def _run_sync_all(self, tool_list, applier_factory, **kwargs):
+        """Helper: patch get_applier + load_local_bundle + skills helpers."""
+
+        def get_applier_side_effect(name):
+            return applier_factory(self.tmpdir, name)
+
+        with (
+            patch("sync_helpers.get_applier", side_effect=get_applier_side_effect),
+            patch("sync_helpers.load_local_bundle", return_value=self.bundle),
+            patch("sync_helpers._resolve_all_mcp_secrets", return_value={}),
+            patch("sync_helpers._discover_installed_skills", return_value=[]),
+            patch("sync_helpers.get_skills_dir", return_value=self.tmpdir / "skills"),
+        ):
+            from sync_helpers import sync_all
+
+            return sync_all(tool_list, **kwargs)
+
+    def test_happy_path_returns_true(self):
+        result = self._run_sync_all(["cursor", "claude-code"], _mock_applier)
+        self.assertTrue(result)
+
+    def test_happy_path_calls_all_three_phases(self):
+        """Each tool's applier should have apply_skills, apply_mcp_servers called."""
+        appliers = {}
+
+        def factory(tmpdir, name):
+            a = _mock_applier(tmpdir, name)
+            appliers[name] = a
+            return a
+
+        self._run_sync_all(["cursor"], factory)
+
+        appliers["cursor"].apply_skills.assert_called_once()
+        appliers["cursor"].apply_mcp_servers.assert_called_once()
+        appliers["cursor"].apply_memory_via_llm.assert_called_once()
+        appliers["cursor"].prune.assert_called_once()
+
+    def test_no_memory_flag_skips_llm(self):
+        appliers = {}
+
+        def factory(tmpdir, name):
+            a = _mock_applier(tmpdir, name)
+            appliers[name] = a
+            return a
+
+        self._run_sync_all(["cursor"], factory, no_memory=True)
+
+        appliers["cursor"].apply_memory_via_llm.assert_not_called()
+
+    def test_partial_failure_returns_true(self):
+        """One tool errors, one succeeds → any_success = True."""
+        call_count = [0]
+
+        def factory(tmpdir, name):
+            call_count[0] += 1
+            if call_count[0] == 1:
+                bad = MagicMock()
+                bad.get_manifest.side_effect = RuntimeError("disk full")
+                return bad
+            return _mock_applier(tmpdir, name)
+
+        result = self._run_sync_all(["bad-tool", "cursor"], factory)
+        self.assertTrue(result)
+
+    def test_all_fail_returns_false(self):
+        """Every tool errors → any_success = False."""
+
+        def factory(tmpdir, name):
+            bad = MagicMock()
+            bad.get_manifest.side_effect = RuntimeError("everything broken")
+            return bad
+
+        result = self._run_sync_all(["cursor", "claude-code"], factory)
+        self.assertFalse(result)
+
+    def test_single_tool_success(self):
+        result = self._run_sync_all(["cursor"], _mock_applier)
+        self.assertTrue(result)
+
+
+class TestSyncSkillsPerToolCounter(unittest.TestCase):
+    """sync_skills: success message must show per-tool counts, not cumulative."""
+
+    def test_per_tool_count_not_cumulative(self):
+        """With 2 tools × 3 skills, the success message for tool-2
+        must say '3 copied' not '6 copied'."""
+        tmpdir = Path(tempfile.mkdtemp())
+        skills = [{"name": f"s{i}", "body": ""} for i in range(3)]
+        success_messages = []
+
+        def factory(tmpdir_inner, name):
+            a = _mock_applier(tmpdir_inner, name)
+            a.apply_skills.return_value = 3
+            a.link_skills.return_value = 0
+            return a
+
+        with (
+            patch("sync_helpers.get_applier", side_effect=lambda n: factory(tmpdir, n)),
+            patch(
+                "sync_helpers.load_local_bundle",
+                return_value={"skills": skills, "mcp_servers": [], "memory": []},
+            ),
+            patch("sync_helpers._discover_installed_skills", return_value=[]),
+            patch("sync_helpers.get_skills_dir", return_value=tmpdir / "skills"),
+            patch("sync_helpers.success", side_effect=lambda msg: success_messages.append(msg)),
+        ):
+            from sync_helpers import sync_skills
+
+            sync_skills(["cursor", "claude-code"])
+
+        # Each message should say 3 copied, not 3 then 6
+        for msg in success_messages:
+            self.assertIn("3 copied", msg, f"Expected '3 copied' in: {msg}")
+            self.assertNotIn("6 copied", msg, f"Unexpected cumulative count in: {msg}")
+
+
+if __name__ == "__main__":
+    unittest.main()