From e49818b0dc0a2b5c85396d9340b771aa9b2a38a9 Mon Sep 17 00:00:00 2001 From: Ace Date: Fri, 6 Mar 2026 13:37:05 -0800 Subject: [PATCH 1/2] fix: status memory crash, sync_skills counter, export loud failure on missing pyrage --- src/export_import.py | 9 +++++--- src/status.py | 6 +++--- src/sync_helpers.py | 14 ++++++++----- tests/test_export_import.py | 41 +++++++++++++++++++++++++++++++++++-- 4 files changed, 57 insertions(+), 13 deletions(-) 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..d2618b7 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 without --no-secrets, export must abort (not silently export 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) From c1a12668dce20ef4ab1da39d00b8bfde1ed3d086 Mon Sep 17 00:00:00 2001 From: Frank Date: Fri, 6 Mar 2026 17:52:51 -0800 Subject: [PATCH 2/2] style: fix E501 line too long in test docstring --- tests/test_export_import.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_export_import.py b/tests/test_export_import.py index d2618b7..4b925c1 100644 --- a/tests/test_export_import.py +++ b/tests/test_export_import.py @@ -311,7 +311,7 @@ def setUp(self): @patch("export_import._check_pyrage", return_value=False) def test_export_fails_loudly_without_pyrage(self, _mock_pyrage): - """Without pyrage and without --no-secrets, export must abort (not silently export plaintext).""" + """Without pyrage and --no-secrets unset, export must abort — no silent plaintext.""" from click.testing import CliRunner runner = CliRunner()