Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/appliers/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ def apply_memory_via_llm(self, collected_memory: List[Dict], manifest: ToolManif
)
else:
warning(f"LLM call failed ({e}), skipping memory sync for {self.TOOL_NAME}")
return 0
return -1 # Signal LLM failure — callers must not show ✓ (#31)

# Parse structured output
try:
Expand Down
21 changes: 21 additions & 0 deletions src/appliers/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ def _empty(self) -> dict:
"schema_version": SCHEMA_VERSION,
"tool": self.tool,
"last_sync_at": None,
"last_sync_result": None,
"skills": {},
"linked_skills": {},
"mcp_servers": {},
Expand All @@ -66,8 +67,28 @@ def is_first_sync(self) -> bool:
"""True when no manifest existed on disk before this run."""
return self._data.get("last_sync_at") is None

@property
def last_sync_failed(self) -> bool:
"""True when the last recorded sync ended with an error."""
return self._data.get("last_sync_result") == "error"

def save(self) -> None:
self._data["last_sync_at"] = _now_iso()
if "last_sync_result" not in self._data or self._data.get("last_sync_result") != "error":
self._data["last_sync_result"] = "success"
self.path.parent.mkdir(parents=True, exist_ok=True)
self.path.write_text(json.dumps(self._data, indent=2), encoding="utf-8")

def save_failure(self, error_msg: str = "") -> None:
"""Persist the manifest with a failure status.

Called when the sync raised an exception so that ``apc status`` can
report the error rather than showing the previous (stale) success.
"""
self._data["last_sync_at"] = _now_iso()
self._data["last_sync_result"] = "error"
if error_msg:
self._data["last_sync_error"] = error_msg
self.path.parent.mkdir(parents=True, exist_ok=True)
self.path.write_text(json.dumps(self._data, indent=2), encoding="utf-8")

Expand Down
4 changes: 4 additions & 0 deletions src/status.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ def _tool_sync_status(name: str) -> str:
if manifest.is_first_sync:
return "not synced"

# If the last sync recorded an error, surface it immediately (#34)
if manifest.last_sync_failed:
return "error"

# Gather all file paths APC last wrote for this tool
recorded_paths: list[str] = []

Expand Down
45 changes: 37 additions & 8 deletions src/sync_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,10 @@ def sync_mcp(tool_list: List[str], override: bool = False) -> int:


def sync_memory(tool_list: List[str]) -> int:
"""Apply memory via LLM transformation to tools. Returns count."""
"""Apply memory via LLM transformation to tools. Returns count.

Returns -1 if every tool's LLM call failed (no ✓ should be shown).
"""
bundle = load_local_bundle()
memory_entries = bundle["memory"]

Expand All @@ -167,20 +170,28 @@ def sync_memory(tool_list: List[str]) -> int:
return 0

total = 0
any_llm_failure = False

for tool_name in tool_list:
try:
applier = get_applier(tool_name)
manifest = applier.get_manifest()

mem = applier.apply_memory_via_llm(memory_entries, manifest)
manifest.save()

total += mem
success(f"{tool_name}: {mem} memory files")
if mem < 0:
# LLM auth/call failed — do not show ✓, do not mark as synced (#31)
any_llm_failure = True
manifest.save_failure(f"LLM call failed for {tool_name}")
error(f"{tool_name}: memory sync failed (LLM unavailable — run 'apc configure')")
else:
manifest.save()
total += mem
success(f"{tool_name}: {mem} memory files")
except Exception as e:
error(f"Failed to sync memory to {tool_name}: {e}")

if any_llm_failure and total == 0:
return -1
return total


Expand Down Expand Up @@ -210,6 +221,7 @@ def sync_all(tool_list: List[str], no_memory: bool = False, override_mcp: bool =
failed_tools = []

for tool_name in tool_list:
manifest = None
try:
applier = get_applier(tool_name)
manifest = applier.get_manifest()
Expand All @@ -224,23 +236,40 @@ def sync_all(tool_list: List[str], no_memory: bool = False, override_mcp: bool =
secrets = _resolve_all_mcp_secrets(mcp_servers)
m = applier.apply_mcp_servers(mcp_servers, secrets, manifest, override=override_mcp)

# Memory
# Memory (-1 means LLM call failed, not a hard error for overall sync)
mem = 0
mem_failed = False
if memory_entries:
mem = applier.apply_memory_via_llm(memory_entries, manifest)
if mem < 0:
mem_failed = True
mem = 0

# Prune orphans
applier.prune(all_skill_names, current_mcp_names, manifest)
manifest.save()

if mem_failed:
# Save manifest but mark memory as failed (#31, #34)
manifest.save_failure(f"LLM memory sync failed for {tool_name}")
mem_label = "memory sync failed"
else:
manifest.save()
mem_label = f"{mem} memory files"

total_skills += s + lk
total_mcp += m
total_memory += mem

success(f"{tool_name}: {s + lk} skills, {m} MCP servers, {mem} memory files")
success(f"{tool_name}: {s + lk} skills, {m} MCP servers, {mem_label}")
except Exception as e:
error(f"Failed to apply to {tool_name}: {e}")
failed_tools.append(tool_name)
# Persist failure so `apc status` reflects the error (#34)
if manifest is not None:
try:
manifest.save_failure(str(e))
except Exception:
pass

any_success = len(failed_tools) < len(tool_list)
if any_success:
Expand Down
2 changes: 2 additions & 0 deletions src/ui.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,8 @@ def tools_status_table(tools: List[Dict[str, str]]) -> None:
badge = Text("⚠ out of sync", style="bold yellow")
elif st == "not synced":
badge = Text("○ not synced", style="yellow")
elif st == "error":
badge = Text("✗ error", style="bold red")
else:
badge = Text("◌ detected", style="dim")
table.add_row(name, badge)
Expand Down
6 changes: 3 additions & 3 deletions tests/test_appliers.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,8 @@ def test_apply_memory_via_llm(self):
content = self.claude_md.read_text()
self.assertIn("Prefers TypeScript", content)

def test_apply_memory_via_llm_returns_zero_on_failure(self):
"""When LLM fails, returns 0 (no fallback to legacy)."""
def test_apply_memory_via_llm_returns_negative_on_failure(self):
"""When LLM fails, returns -1 to signal failure (not 0 which looks like success)."""
collected = [
{"id": "abc", "source_tool": "openclaw", "content": "test"},
]
Expand All @@ -159,7 +159,7 @@ def test_apply_memory_via_llm_returns_zero_on_failure(self):
applier = ClaudeApplier()
count = applier.apply_memory_via_llm(collected, manifest)

self.assertEqual(count, 0)
self.assertEqual(count, -1) # -1 signals LLM failure, not zero-files-written

def test_apply_memory_via_llm_handles_markdown_fencing(self):
"""LLM sometimes wraps response in markdown code blocks."""
Expand Down
2 changes: 1 addition & 1 deletion tests/test_e2e.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def test_no_llm_configured_shows_warning(self):
applier = ClaudeApplier()
count = applier.apply_memory_via_llm(collected, manifest)

self.assertEqual(count, 0)
self.assertEqual(count, -1) # -1 signals LLM failure


if __name__ == "__main__":
Expand Down
4 changes: 2 additions & 2 deletions tests/test_security_llm_memory_path.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ def _read_existing_memory_files(self):
result = applier.apply_memory_via_llm(
[{"id": "x", "source_tool": "t", "content": "hello"}], manifest
)
# Returns 0 due to LLM exception, not RuntimeError
self.assertEqual(result, 0)
# Returns -1 due to LLM exception (signals failure), not RuntimeError
self.assertEqual(result, -1)


class TestExpandUserInLLMWritePath(unittest.TestCase):
Expand Down
Loading