From ba1593ea0f40ca2f8e2b192c07a3996504881584 Mon Sep 17 00:00:00 2001 From: JeremyDev87 Date: Sat, 4 Apr 2026 00:08:42 +0900 Subject: [PATCH] fix(plugin): unify data dir resolver and fix standalone diagnostics (#1225) Centralize plugin data directory resolution into hooks/lib/data_dir.py so all modules respect CLAUDE_PLUGIN_DATA env var instead of hardcoding ~/.codingbuddy. Also fix health_check standalone diagnostics to treat missing .ai-rules/ as PASS (ModeEngine has template fallback). --- .../hooks/lib/agent_memory.py | 9 +-- .../claude-code-plugin/hooks/lib/data_dir.py | 16 ++++++ .../hooks/lib/event_bridge.py | 8 ++- .../hooks/lib/health_check.py | 19 +++++-- .../hooks/lib/history_db.py | 14 +++-- .../hooks/lib/onboarding_tour.py | 17 ++++-- .../claude-code-plugin/tests/test_data_dir.py | 57 +++++++++++++++++++ .../tests/test_health_check.py | 8 ++- .../tests/test_onboarding_tour.py | 18 +++--- 9 files changed, 131 insertions(+), 35 deletions(-) create mode 100644 packages/claude-code-plugin/hooks/lib/data_dir.py create mode 100644 packages/claude-code-plugin/tests/test_data_dir.py diff --git a/packages/claude-code-plugin/hooks/lib/agent_memory.py b/packages/claude-code-plugin/hooks/lib/agent_memory.py index dddf5660..5cf94e29 100644 --- a/packages/claude-code-plugin/hooks/lib/agent_memory.py +++ b/packages/claude-code-plugin/hooks/lib/agent_memory.py @@ -1,19 +1,20 @@ -"""AgentMemory — per-agent persistent knowledge across sessions (#947). +"""AgentMemory -- per-agent persistent knowledge across sessions (#947). Stores findings, patterns, and preferences per agent in JSON files -under ~/.codingbuddy/agent_memory/. +under /agent_memory/. """ import json import os from typing import Optional +from data_dir import resolve_data_dir + class AgentMemory: - DEFAULT_DIR = os.path.expanduser("~/.codingbuddy/agent_memory") MAX_ITEMS = 50 def __init__(self, memory_dir: Optional[str] = None, max_items: int = MAX_ITEMS): - self.memory_dir = memory_dir or self.DEFAULT_DIR + self.memory_dir = memory_dir or os.path.join(resolve_data_dir(), "agent_memory") self.max_items = max_items def _filepath(self, agent_name: str) -> str: diff --git a/packages/claude-code-plugin/hooks/lib/data_dir.py b/packages/claude-code-plugin/hooks/lib/data_dir.py new file mode 100644 index 00000000..9624db39 --- /dev/null +++ b/packages/claude-code-plugin/hooks/lib/data_dir.py @@ -0,0 +1,16 @@ +"""Centralized plugin data directory resolver. + +All plugin modules should use resolve_data_dir() instead of hardcoding ~/.codingbuddy. +Respects CLAUDE_PLUGIN_DATA environment variable. +""" +import os + +DEFAULT_DATA_DIR = os.path.join(os.path.expanduser("~"), ".codingbuddy") + + +def resolve_data_dir() -> str: + """Resolve plugin data directory. + + Resolution: CLAUDE_PLUGIN_DATA env -> ~/.codingbuddy (default). + """ + return os.environ.get("CLAUDE_PLUGIN_DATA", DEFAULT_DATA_DIR) diff --git a/packages/claude-code-plugin/hooks/lib/event_bridge.py b/packages/claude-code-plugin/hooks/lib/event_bridge.py index 5c68c65f..1c64ff9a 100644 --- a/packages/claude-code-plugin/hooks/lib/event_bridge.py +++ b/packages/claude-code-plugin/hooks/lib/event_bridge.py @@ -1,12 +1,14 @@ -"""File-based event bridge for Plugin(Python) → MCP(TypeScript) communication. +"""File-based event bridge for Plugin(Python) -> MCP(TypeScript) communication. -Emits events as JSON lines to ~/.codingbuddy/events/.jsonl. +Emits events as JSON lines to /events/.jsonl. """ import json import os from datetime import datetime, timezone from typing import Optional +from data_dir import resolve_data_dir + EVENT_TYPES = ( "tool_call", "session_start", @@ -22,7 +24,7 @@ class EventBridge: def __init__(self, session_id: str, events_dir: Optional[str] = None): self.session_id = session_id self.events_dir = events_dir or os.path.join( - os.path.expanduser("~"), ".codingbuddy", "events" + resolve_data_dir(), "events" ) @property diff --git a/packages/claude-code-plugin/hooks/lib/health_check.py b/packages/claude-code-plugin/hooks/lib/health_check.py index c8fc11f5..9eb576bc 100644 --- a/packages/claude-code-plugin/hooks/lib/health_check.py +++ b/packages/claude-code-plugin/hooks/lib/health_check.py @@ -11,6 +11,7 @@ from typing import Dict, List, Optional sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) +from data_dir import resolve_data_dir from runtime_mode import detect_runtime_mode HOOK_FILES = [ @@ -42,7 +43,9 @@ def __init__( self._home_dir = home_dir or os.path.expanduser("~") self._project_dir = project_dir or plugin_root self._hooks_dir = os.path.join(plugin_root, "hooks") - self._data_dir = os.path.join(self._home_dir, ".codingbuddy") + self._data_dir = ( + os.path.join(self._home_dir, ".codingbuddy") if home_dir else resolve_data_dir() + ) self._claude_dir = os.path.join(self._home_dir, ".claude") # ------------------------------------------------------------------ @@ -202,13 +205,13 @@ def check_runtime_mode(self) -> Dict[str, str]: def check_standalone_readiness(self) -> Dict[str, str]: """Check if standalone mode prerequisites are met.""" issues = [] - # Check .ai-rules existence + has_ai_rules = True + # Check .ai-rules existence (informational only — ModeEngine has template fallback) rules_dir = os.path.join(self._project_dir, ".ai-rules") if not os.path.isdir(rules_dir): - # Also check packages path pkg_rules = os.path.join(self._plugin_root, "..", "rules", ".ai-rules") if not os.path.isdir(os.path.normpath(pkg_rules)): - issues.append(".ai-rules/ not found") + has_ai_rules = False # Check ModeEngine importable try: from mode_engine import ModeEngine # noqa: F401 @@ -220,7 +223,13 @@ def check_standalone_readiness(self) -> Dict[str, str]: issues.append("UserPromptSubmit hook not registered") if not issues: - return _result("standalone_readiness", "PASS", "Standalone mode ready") + if has_ai_rules: + return _result("standalone_readiness", "PASS", "Standalone mode ready") + return _result( + "standalone_readiness", + "PASS", + "Standalone mode ready (template fallback, no .ai-rules)", + ) return _result("standalone_readiness", "WARN", f"Standalone not ready: {', '.join(issues)}") # ------------------------------------------------------------------ diff --git a/packages/claude-code-plugin/hooks/lib/history_db.py b/packages/claude-code-plugin/hooks/lib/history_db.py index 6abda73d..7dd798c2 100644 --- a/packages/claude-code-plugin/hooks/lib/history_db.py +++ b/packages/claude-code-plugin/hooks/lib/history_db.py @@ -5,9 +5,13 @@ import stat import time +from data_dir import resolve_data_dir + logger = logging.getLogger(__name__) -DEFAULT_DB_PATH = os.path.expanduser("~/.codingbuddy/history.db") + +def _default_db_path() -> str: + return os.path.join(resolve_data_dir(), "history.db") class HistoryDB: @@ -16,10 +20,10 @@ class HistoryDB: _instance: "HistoryDB | None" = None @classmethod - def get_instance(cls, db_path: str = DEFAULT_DB_PATH) -> "HistoryDB": + def get_instance(cls, db_path: str = None) -> "HistoryDB": """Return the singleton instance, creating it on first call.""" if cls._instance is None: - cls._instance = cls(db_path=db_path) + cls._instance = cls(db_path=db_path or _default_db_path()) return cls._instance @classmethod @@ -29,8 +33,8 @@ def close_instance(cls) -> None: cls._instance.close() cls._instance = None - def __init__(self, db_path: str = DEFAULT_DB_PATH): - self._db_path = db_path + def __init__(self, db_path: str = None): + self._db_path = db_path or _default_db_path() self._ensure_directory() self._conn = sqlite3.connect(db_path, timeout=10) self._conn.execute("PRAGMA journal_mode=WAL") diff --git a/packages/claude-code-plugin/hooks/lib/onboarding_tour.py b/packages/claude-code-plugin/hooks/lib/onboarding_tour.py index a7aa9b28..8b8eaf8a 100644 --- a/packages/claude-code-plugin/hooks/lib/onboarding_tour.py +++ b/packages/claude-code-plugin/hooks/lib/onboarding_tour.py @@ -15,10 +15,15 @@ render_face_banner, render_section_header, ) +from data_dir import resolve_data_dir -# Flag file location -ONBOARDED_DIR = os.path.join(os.path.expanduser("~"), ".codingbuddy") -ONBOARDED_FLAG = os.path.join(ONBOARDED_DIR, "onboarded") + +def _onboarded_dir() -> str: + return resolve_data_dir() + + +def _onboarded_flag() -> str: + return os.path.join(_onboarded_dir(), "onboarded") # Environment variable to skip tour SKIP_ENV_VAR = "CODINGBUDDY_SKIP_TOUR" @@ -32,13 +37,13 @@ def is_first_run() -> bool: """ if os.environ.get(SKIP_ENV_VAR): return False - return not os.path.isfile(ONBOARDED_FLAG) + return not os.path.isfile(_onboarded_flag()) def mark_onboarded() -> None: """Create the onboarded flag file to prevent future tours.""" - os.makedirs(ONBOARDED_DIR, exist_ok=True) - Path(ONBOARDED_FLAG).touch() + os.makedirs(_onboarded_dir(), exist_ok=True) + Path(_onboarded_flag()).touch() # ── i18n Tour Content ────────────────────────────────────────────── diff --git a/packages/claude-code-plugin/tests/test_data_dir.py b/packages/claude-code-plugin/tests/test_data_dir.py new file mode 100644 index 00000000..3fa85a47 --- /dev/null +++ b/packages/claude-code-plugin/tests/test_data_dir.py @@ -0,0 +1,57 @@ +"""Tests for data_dir — centralized plugin data directory resolver (#1225).""" +import os +import sys + +import pytest + +_tests_dir = os.path.dirname(os.path.abspath(__file__)) +_lib_dir = os.path.join(os.path.dirname(_tests_dir), "hooks", "lib") +if _lib_dir not in sys.path: + sys.path.insert(0, _lib_dir) + +from data_dir import DEFAULT_DATA_DIR, resolve_data_dir + + +class TestResolveDataDir: + def test_returns_custom_path_when_env_set(self, monkeypatch, tmp_path): + custom = str(tmp_path / "custom_data") + monkeypatch.setenv("CLAUDE_PLUGIN_DATA", custom) + assert resolve_data_dir() == custom + + def test_returns_default_when_env_unset(self, monkeypatch): + monkeypatch.delenv("CLAUDE_PLUGIN_DATA", raising=False) + result = resolve_data_dir() + expected = os.path.join(os.path.expanduser("~"), ".codingbuddy") + assert result == expected + + def test_default_data_dir_constant(self): + assert DEFAULT_DATA_DIR == os.path.join( + os.path.expanduser("~"), ".codingbuddy" + ) + + +class TestModulesUseResolveDataDir: + """Verify that modules import and use resolve_data_dir.""" + + def test_history_db_uses_resolve(self, monkeypatch, tmp_path): + custom = str(tmp_path / "hist") + monkeypatch.setenv("CLAUDE_PLUGIN_DATA", custom) + from history_db import _default_db_path + + assert _default_db_path() == os.path.join(custom, "history.db") + + def test_event_bridge_uses_resolve(self, monkeypatch, tmp_path): + custom = str(tmp_path / "evt") + monkeypatch.setenv("CLAUDE_PLUGIN_DATA", custom) + from event_bridge import EventBridge + + bridge = EventBridge(session_id="test") + assert bridge.events_dir == os.path.join(custom, "events") + + def test_agent_memory_uses_resolve(self, monkeypatch, tmp_path): + custom = str(tmp_path / "mem") + monkeypatch.setenv("CLAUDE_PLUGIN_DATA", custom) + from agent_memory import AgentMemory + + am = AgentMemory() + assert am.memory_dir == os.path.join(custom, "agent_memory") diff --git a/packages/claude-code-plugin/tests/test_health_check.py b/packages/claude-code-plugin/tests/test_health_check.py index 22fae836..c5df9bee 100644 --- a/packages/claude-code-plugin/tests/test_health_check.py +++ b/packages/claude-code-plugin/tests/test_health_check.py @@ -276,11 +276,13 @@ def test_ready_with_all_requirements(self, env): # ModeEngine may not be importable in test env, so just check it runs assert result["status"] in ("PASS", "WARN") - def test_missing_ai_rules(self, env): + def test_missing_ai_rules_passes_with_fallback(self, env): checker = _make_checker(env) result = checker.check_standalone_readiness() - assert result["status"] == "WARN" - assert ".ai-rules/" in result["message"] + # .ai-rules missing is no longer WARN — ModeEngine has template fallback + assert result["status"] in ("PASS", "WARN") + if result["status"] == "PASS": + assert "template fallback" in result["message"] class TestRunAll: diff --git a/packages/claude-code-plugin/tests/test_onboarding_tour.py b/packages/claude-code-plugin/tests/test_onboarding_tour.py index be7b075f..f09f2bc2 100644 --- a/packages/claude-code-plugin/tests/test_onboarding_tour.py +++ b/packages/claude-code-plugin/tests/test_onboarding_tour.py @@ -26,7 +26,7 @@ def test_returns_true_when_flag_missing(self): """First run when no onboarded flag exists.""" with tempfile.TemporaryDirectory() as tmpdir: flag = os.path.join(tmpdir, "onboarded") - with patch.object(onboarding_tour, "ONBOARDED_FLAG", flag): + with patch.object(onboarding_tour, "_onboarded_flag", return_value=flag): with patch.dict(os.environ, {}, clear=False): os.environ.pop(onboarding_tour.SKIP_ENV_VAR, None) assert onboarding_tour.is_first_run() is True @@ -36,7 +36,7 @@ def test_returns_false_when_flag_exists(self): with tempfile.TemporaryDirectory() as tmpdir: flag = os.path.join(tmpdir, "onboarded") Path(flag).touch() - with patch.object(onboarding_tour, "ONBOARDED_FLAG", flag): + with patch.object(onboarding_tour, "_onboarded_flag", return_value=flag): with patch.dict(os.environ, {}, clear=False): os.environ.pop(onboarding_tour.SKIP_ENV_VAR, None) assert onboarding_tour.is_first_run() is False @@ -45,7 +45,7 @@ def test_returns_false_when_skip_env_set(self): """Skip tour when CODINGBUDDY_SKIP_TOUR env var is set.""" with tempfile.TemporaryDirectory() as tmpdir: flag = os.path.join(tmpdir, "onboarded") - with patch.object(onboarding_tour, "ONBOARDED_FLAG", flag): + with patch.object(onboarding_tour, "_onboarded_flag", return_value=flag): with patch.dict(os.environ, {onboarding_tour.SKIP_ENV_VAR: "1"}): assert onboarding_tour.is_first_run() is False @@ -58,8 +58,8 @@ def test_creates_flag_file(self): with tempfile.TemporaryDirectory() as tmpdir: flag_dir = os.path.join(tmpdir, ".codingbuddy") flag = os.path.join(flag_dir, "onboarded") - with patch.object(onboarding_tour, "ONBOARDED_DIR", flag_dir): - with patch.object(onboarding_tour, "ONBOARDED_FLAG", flag): + with patch.object(onboarding_tour, "_onboarded_dir", return_value=flag_dir): + with patch.object(onboarding_tour, "_onboarded_flag", return_value=flag): onboarding_tour.mark_onboarded() assert os.path.isfile(flag) @@ -68,8 +68,8 @@ def test_creates_parent_directory(self): with tempfile.TemporaryDirectory() as tmpdir: flag_dir = os.path.join(tmpdir, "nested", ".codingbuddy") flag = os.path.join(flag_dir, "onboarded") - with patch.object(onboarding_tour, "ONBOARDED_DIR", flag_dir): - with patch.object(onboarding_tour, "ONBOARDED_FLAG", flag): + with patch.object(onboarding_tour, "_onboarded_dir", return_value=flag_dir): + with patch.object(onboarding_tour, "_onboarded_flag", return_value=flag): onboarding_tour.mark_onboarded() assert os.path.isdir(flag_dir) assert os.path.isfile(flag) @@ -79,8 +79,8 @@ def test_idempotent(self): with tempfile.TemporaryDirectory() as tmpdir: flag_dir = os.path.join(tmpdir, ".codingbuddy") flag = os.path.join(flag_dir, "onboarded") - with patch.object(onboarding_tour, "ONBOARDED_DIR", flag_dir): - with patch.object(onboarding_tour, "ONBOARDED_FLAG", flag): + with patch.object(onboarding_tour, "_onboarded_dir", return_value=flag_dir): + with patch.object(onboarding_tour, "_onboarded_flag", return_value=flag): onboarding_tour.mark_onboarded() onboarding_tour.mark_onboarded() # Should not raise assert os.path.isfile(flag)