diff --git a/src/export_import.py b/src/export_import.py index 6741bef..9bc5e9b 100644 --- a/src/export_import.py +++ b/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() diff --git a/src/status.py b/src/status.py index 4e4ba07..c228bae 100644 --- a/src/status.py +++ b/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: diff --git a/src/sync_helpers.py b/src/sync_helpers.py index 7622b26..759492d 100644 --- a/src/sync_helpers.py +++ b/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}") diff --git a/tests/test_export_import.py b/tests/test_export_import.py index c5d1e31..4b925c1 100644 --- a/tests/test_export_import.py +++ b/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) diff --git a/tests/test_manifest.py b/tests/test_manifest.py index 2e7e341..12b57c5 100644 --- a/tests/test_manifest.py +++ b/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" diff --git a/tests/test_sync_helpers.py b/tests/test_sync_helpers.py new file mode 100644 index 0000000..4e60288 --- /dev/null +++ b/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()