Skip to content

Commit 8d8b759

Browse files
committed
fix: CI failures — os.readlink crash, skills.json index, dir_sync manifest + status check
- base.py: fix OSError in sync_skills_dir() — os.readlink() was called before is_symlink() check; restructured to guard correctly - collect.py: restore save_skills() for index (metadata only, no body) so apc skill list and existing tests still work - manifest.py: add record_dir_sync() + dir_sync field in schema - sync_helpers.py: call manifest.record_dir_sync() after sync_skills_dir() succeeds - status.py: check dir_sync symlink integrity for exclusive-dir tools - test_docker_integration.py: seed ~/.apc/skills/ instead of inline skills.json body; update test_out_of_sync to break dir symlink for exclusive-dir tools
1 parent cdce52a commit 8d8b759

6 files changed

Lines changed: 65 additions & 24 deletions

File tree

src/appliers/base.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,8 +163,10 @@ def sync_skills_dir(self) -> bool:
163163
skill_dir = self.SKILL_DIR
164164

165165
# Already correctly symlinked — nothing to do
166-
link_target = Path(os.readlink(skill_dir)).resolve()
167-
if skill_dir.is_symlink() and link_target == skills_source.resolve():
166+
already_linked = skill_dir.is_symlink() and (
167+
Path(os.readlink(skill_dir)).resolve() == skills_source.resolve()
168+
)
169+
if already_linked:
168170
return True
169171

170172
# Remove whatever is there now

src/appliers/manifest.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,19 @@ def _empty(self) -> dict:
5757
"last_sync_at": None,
5858
"skills": {},
5959
"linked_skills": {},
60+
"dir_sync": None,
6061
"mcp_servers": {},
6162
"memory": {},
6263
}
6364

65+
def record_dir_sync(self, skill_dir: str, target: str) -> None:
66+
"""Record a dir-level symlink: skill_dir → target (~/.apc/skills/)."""
67+
self._data["dir_sync"] = {
68+
"skill_dir": skill_dir,
69+
"target": target,
70+
"synced_at": _now_iso(),
71+
}
72+
6473
@property
6574
def is_first_sync(self) -> bool:
6675
"""True when no manifest existed on disk before this run."""

src/collect.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,13 @@
1212
from cache import (
1313
load_mcp_servers,
1414
load_memory,
15+
load_skills,
1516
merge_mcp_servers,
1617
merge_memory,
18+
merge_skills,
1719
save_mcp_servers,
1820
save_memory,
21+
save_skills,
1922
)
2023
from extractors import detect_installed_tools, get_extractor
2124
from frontmatter_parser import render_frontmatter
@@ -175,9 +178,18 @@ def collect(tools, no_memory, yes):
175178

176179
merged_mcp = merge_mcp_servers(load_mcp_servers(), new_mcp_servers)
177180
merged_memory = merge_memory(load_memory(), selected_memory)
181+
# Keep skills.json as a metadata index (name, description, tags — no body)
182+
# so `apc skill list` and other commands can enumerate skills without
183+
# reading every SKILL.md. Body lives in ~/.apc/skills/<name>/SKILL.md.
184+
skill_index = [
185+
{k: s[k] for k in ("name", "description", "tags", "version", "source_tool") if k in s}
186+
for s in new_skills
187+
]
188+
merged_index = merge_skills(load_skills(), skill_index)
178189

179190
save_mcp_servers(merged_mcp)
180191
save_memory(merged_memory)
192+
save_skills(merged_index)
181193

182194
cache_summary_table(
183195
len(new_skills), len(merged_mcp), len(merged_memory), title="Local Cache Updated"

src/status.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
No login required. No network calls.
44
"""
55

6+
import os
67
from pathlib import Path
78

89
import click
@@ -46,6 +47,14 @@ def _tool_sync_status(name: str) -> str:
4647
if fp := info_dict.get("link_path"):
4748
recorded_paths.append(fp)
4849

50+
# Dir-level symlink check: verify the skills dir still points to ~/.apc/skills/
51+
if dir_sync := manifest._data.get("dir_sync"):
52+
skill_dir = Path(dir_sync.get("skill_dir", ""))
53+
target = Path(dir_sync.get("target", ""))
54+
# Symlink must exist and still resolve to the canonical source
55+
if not skill_dir.is_symlink() or Path(os.readlink(skill_dir)).resolve() != target.resolve():
56+
return "out of sync"
57+
4958
# memory is a flat dict {file_path, checksum, ...}, not a dict-of-dicts.
5059
if fp := manifest._data.get("memory", {}).get("file_path"):
5160
recorded_paths.append(fp)

src/sync_helpers.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ def sync_skills(tool_list: List[str]) -> Tuple[int, int]:
9292

9393
if applier.sync_skills_dir():
9494
# Dir-level symlink established — entire ~/.apc/skills/ is live
95+
manifest.record_dir_sync(str(applier.SKILL_DIR), str(skills_dir))
9596
total_dir += 1
9697
success(f"{tool_name}: skills dir symlinked → ~/.apc/skills/")
9798
elif installed_skills:
@@ -202,6 +203,7 @@ def sync_all(tool_list: List[str], no_memory: bool = False, override_mcp: bool =
202203

203204
# Establish skill link (dir-level or per-skill depending on tool)
204205
if applier.sync_skills_dir():
206+
manifest.record_dir_sync(str(applier.SKILL_DIR), str(skills_dir))
205207
s, lk = 1, 0 # dir symlink counts as 1
206208
else:
207209
s = 0

tests/test_docker_integration.py

Lines changed: 29 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -406,29 +406,27 @@ def test_out_of_sync_when_skill_deleted(
406406
mcp_key,
407407
supports_skills,
408408
):
409-
"""Deleting a synced skill file causes 'out of sync' for that tool."""
409+
"""Removing a synced skill link/file causes 'out of sync' for that tool.
410+
411+
Skills now live in ~/.apc/skills/ (source of truth).
412+
- SKILL_DIR_EXCLUSIVE tools (claude-code, openclaw): their skills dir is a
413+
single symlink → ~/.apc/skills/. Out-of-sync = symlink removed/broken.
414+
- Per-skill tools (cursor): individual link_path entries in the manifest.
415+
Out-of-sync = linked file deleted.
416+
"""
410417
monkeypatch.setenv("HOME", str(tmp_path))
411418
monkeypatch.chdir(tmp_path)
412419
for d in seed_dirs:
413420
(tmp_path / d).mkdir(parents=True, exist_ok=True)
414421

415-
# Seed isolated cache then sync — appliers are lazy so all writes go to tmp_path
422+
# Seed ~/.apc/skills/test-skill/SKILL.md — the canonical source
423+
skills_source = tmp_path / ".apc" / "skills" / "test-skill"
424+
skills_source.mkdir(parents=True, exist_ok=True)
425+
(skills_source / "SKILL.md").write_text("# Test\nTest skill body.", encoding="utf-8")
426+
416427
cache_dir = tmp_path / ".apc" / "cache"
417428
cache_dir.mkdir(parents=True, exist_ok=True)
418-
(cache_dir / "skills.json").write_text(
419-
json.dumps(
420-
[
421-
{
422-
"name": "test-skill",
423-
"description": "A test skill",
424-
"body": "Test skill body.",
425-
"tags": ["test"],
426-
"version": "1.0.0",
427-
}
428-
]
429-
),
430-
encoding="utf-8",
431-
)
429+
(cache_dir / "skills.json").write_text("[]", encoding="utf-8")
432430
(cache_dir / "mcp_servers.json").write_text("[]", encoding="utf-8")
433431
(cache_dir / "memory.json").write_text("[]", encoding="utf-8")
434432
runner.invoke(cli, ["sync", "--tools", detected_name, "--yes", "--no-memory"])
@@ -439,14 +437,23 @@ def test_out_of_sync_when_skill_deleted(
439437
f"{detected_name}: expected synced before delete, got:\n{r1.output}"
440438
)
441439

442-
# Find and delete a skill file recorded by the manifest
440+
# Break the skill link/symlink to trigger out-of-sync
443441
manifest_path = tmp_path / ".apc" / "manifests" / f"{applier_tool_name}.json"
444442
manifest_data = json.loads(manifest_path.read_text())
445-
skill_paths = [
446-
v["file_path"] for v in manifest_data.get("skills", {}).values() if "file_path" in v
447-
]
448-
assert skill_paths, f"no skill file_paths in manifest for {detected_name}"
449-
Path(skill_paths[0]).unlink()
443+
444+
if manifest_data.get("dir_sync"):
445+
# Exclusive-dir tool: remove the dir-level symlink
446+
skill_dir = Path(manifest_data["dir_sync"]["skill_dir"])
447+
skill_dir.unlink()
448+
else:
449+
# Per-skill tool: delete a linked skill file
450+
link_paths = [
451+
v["link_path"]
452+
for v in manifest_data.get("linked_skills", {}).values()
453+
if "link_path" in v
454+
]
455+
assert link_paths, f"no link_paths in manifest for {detected_name}"
456+
Path(link_paths[0]).unlink()
450457

451458
r2 = runner.invoke(cli, ["status"])
452459
assert "out of sync" in r2.output.lower(), (

0 commit comments

Comments
 (0)