diff --git a/README.md b/README.md index bb04546..a4e3eaf 100644 --- a/README.md +++ b/README.md @@ -85,7 +85,6 @@ agent-notes [options] |---------|-------------| | `install [--local] [--copy]` | Interactive wizard or direct install | | `uninstall [--local]` | Remove installed components | -| `update` | Pull latest, rebuild, reinstall | | `doctor [--local] [--fix]` | Check installation health | | `info` | Show status and component counts | | `list [clis\|models\|roles\|agents\|skills\|rules\|all]` | List engine components or installed | diff --git a/agent_notes/cli.py b/agent_notes/cli.py index 8d0b4af..82d9a0f 100644 --- a/agent_notes/cli.py +++ b/agent_notes/cli.py @@ -237,15 +237,6 @@ def main(): p_uninstall = subparsers.add_parser("uninstall", help="Remove installed components") p_uninstall.add_argument("--local", action="store_true", help="Remove from current project") - # update - p_update = subparsers.add_parser("update", help="Pull latest, show diff, reinstall") - p_update.add_argument("--dry-run", action="store_true", help="Show diff only, do not reinstall") - p_update.add_argument("-y", "--yes", action="store_true", help="Skip confirmation prompt") - p_update.add_argument("--only", action="append", choices=["agents","skills","rules","commands","config","settings"], - help="Filter diff to these component types (repeatable)") - p_update.add_argument("--since", help="Override 'before' commit label (cosmetic only for now)") - p_update.add_argument("--skip-pull", action="store_true", help="Skip git pull") - # doctor p_doctor = subparsers.add_parser("doctor", help="Check installation health") p_doctor.add_argument("--local", action="store_true", help="Check local installation") @@ -325,15 +316,6 @@ def main(): elif args.command == "uninstall": from .commands.install import uninstall uninstall(local=args.local) - elif args.command == "update": - from .commands.update import update - update( - dry_run=args.dry_run, - yes=args.yes, - only=args.only, - since=args.since, - skip_pull=args.skip_pull, - ) elif args.command == "doctor": from .commands.doctor import doctor doctor(local=args.local, fix=args.fix) diff --git a/agent_notes/commands/__init__.py b/agent_notes/commands/__init__.py index b62ae78..4d36870 100644 --- a/agent_notes/commands/__init__.py +++ b/agent_notes/commands/__init__.py @@ -14,14 +14,13 @@ from . import build from . import doctor from . import validate -from . import update from . import regenerate from . import list as list_cmd from . import memory as memory_cmd __all__ = [ "install", "uninstall", "show_info", - "build", "doctor", "validate", "update", + "build", "doctor", "validate", "regenerate", "set_role", "interactive_install", "list_cmd", "memory_cmd", ] \ No newline at end of file diff --git a/agent_notes/commands/doctor.py b/agent_notes/commands/doctor.py index dafab85..9e4126b 100644 --- a/agent_notes/commands/doctor.py +++ b/agent_notes/commands/doctor.py @@ -62,25 +62,60 @@ def _check_session_hook(scope: str, issues: list) -> None: )) +def check_version_drift(scope: str, issues: list, fix_actions: list) -> None: + """Check if the installed package version matches the current running version.""" + from .. import install_state + from ..config import get_version + from ..domain.diagnostics import Issue, FixAction + from ..services.state_store import get_scope + from pathlib import Path + + state = install_state.load_current_state() + if state is None: + return + + project_path = Path.cwd() if scope == "local" else None + scope_state = get_scope(state, scope, project_path) + if scope_state is None: + return + + installed_version = scope_state.installed_version + if not installed_version: + return + + current_version = get_version() + if installed_version != current_version: + issues.append(Issue( + "version_drift", + "state.json", + f"Installed with v{installed_version} but running v{current_version}. " + "Run `agent-notes doctor --fix` or `agent-notes install` to update.", + )) + fix_actions.append(FixAction("_TRIGGER_INSTALL", "state.json", "reinstall to update")) + + def diagnose(scope: str, fix: bool = False) -> bool: """Run all diagnostic checks and optionally apply fixes.""" from .. import install_state - + print_summary(scope) - + issues = [] fix_actions = [] - + # Run checks check_stale_files(scope, issues, fix_actions) - check_broken_symlinks(scope, issues, fix_actions) + check_broken_symlinks(scope, issues, fix_actions) check_shadowed_files(scope, issues, fix_actions) check_missing_files(scope, issues, fix_actions) check_content_drift(scope, issues, fix_actions) - + # Build freshness check (scope-independent) check_build_freshness(issues, fix_actions) + # Version drift check + check_version_drift(scope, issues, fix_actions) + # SessionStart hook check (Claude Code only) _check_session_hook(scope, issues) diff --git a/agent_notes/commands/update.py b/agent_notes/commands/update.py deleted file mode 100644 index 3a75631..0000000 --- a/agent_notes/commands/update.py +++ /dev/null @@ -1,169 +0,0 @@ -"""Pull latest changes, rebuild, show diff, and reinstall.""" - -from __future__ import annotations - -import subprocess -from pathlib import Path -from typing import Optional - -from ..config import ROOT, Color, get_version, PKG_DIR -from .. import install_state -from ..services import diff as update_diff - - -def _run_git(args, cwd) -> subprocess.CompletedProcess: - return subprocess.run( - ["git", *args], cwd=cwd, - capture_output=True, text=True, check=True, - ) - - -def _git_head(repo) -> str: - try: - return _run_git(["rev-parse", "HEAD"], repo).stdout.strip() - except Exception: - return "" - - -def _git_pull(repo) -> None: - _run_git(["pull", "--ff-only"], repo) - - -def _show_commits(repo, before: str, after: str, limit: int = 5) -> None: - if not before or not after or before == after: - return - try: - out = _run_git(["log", "--oneline", f"{before}..{after}"], repo).stdout.strip() - except Exception: - return - if not out: - return - lines = out.split("\n") - print(f"{Color.GREEN}Updated{Color.NC} {len(lines)} commits.") - for line in lines[:limit]: - print(f" {line}") - if len(lines) > limit: - print(f" ... and {len(lines) - limit} more") - print("") - - -def update( - dry_run: bool = False, - yes: bool = False, - only: Optional[list[str]] = None, - since: Optional[str] = None, - skip_pull: bool = False, -) -> None: - """Pull, rebuild, diff against state.json, prompt, reinstall. - - - dry_run: show the diff, do NOT reinstall - - yes: don't prompt, just reinstall if there are changes - - only: list of component types to include in the diff (agents, skills, rules, commands, config, settings) - - since: if set, compare against this git sha rather than current state.json (advanced) - - skip_pull: skip the git pull (useful when user already pulled) - """ - repo = ROOT - print("Updating agent-notes...") - print("") - - # Step 1: git pull - if not skip_pull: - git_dir = repo / ".git" - if not git_dir.exists(): - print(f"{Color.RED}Error:{Color.NC} Not a git repository. Update requires a git-based install.") - return - before = _git_head(repo) - try: - _git_pull(repo) - except subprocess.CalledProcessError: - print(f"{Color.RED}Error:{Color.NC} Could not fast-forward. Resolve manually: cd {repo} && git status") - return - after = _git_head(repo) - _show_commits(repo, before, after) - if before == after and before: - print(f"{Color.GREEN}Already up to date (no new commits).{Color.NC}") - - # Step 2: rebuild dist/ - print("Rebuilding...") - try: - from ..commands.build import build as run_build - run_build() - except Exception as e: - print(f"{Color.RED}Build failed: {e}{Color.NC}") - return - - # Step 3: determine which scope to update and compute "new" state - old_state = install_state.load_current_state() - - # Determine scope: if CWD has a local install, update that; otherwise update global - current_project = Path.cwd() - local_exists = old_state and str(current_project.resolve()) in old_state.local_installs if old_state else False - global_exists = old_state and old_state.global_install is not None if old_state else False - - # Default to global unless local exists and global doesn't, or if only local exists - if local_exists and not global_exists: - scope = "local" - project_path = current_project - elif local_exists and global_exists: - # Both exist - default to global (could add --local flag in future) - scope = "global" - project_path = None - else: - # Default to global - scope = "global" - project_path = None - - # Get the existing scope's mode, or default - if scope == "global" and old_state and old_state.global_install: - mode = old_state.global_install.mode - elif scope == "local" and old_state and str(current_project.resolve()) in old_state.local_installs: - mode = old_state.local_installs[str(current_project.resolve())].mode - else: - mode = "symlink" # default - - new_state = install_state.build_install_state( - mode=mode, - scope=scope, - repo_root=PKG_DIR.parent, - project_path=project_path, - ) - - # If `since` is provided, we'd need to stash old state and rebuild from that commit. - # Keep it minimal for now: `since` only influences the commit label in the diff output. - if since: - if old_state is not None: - old_state.source_commit = since - else: - print(f"{Color.YELLOW}Warning: --since provided but no prior state.json; treating as initial install.{Color.NC}") - - # Step 4: diff - diff = update_diff.diff_states(old_state, new_state) - if only: - diff = update_diff.filter_diff(diff, only=only) - - # Step 5: render report - print("") - print(update_diff.render_diff_report(diff, use_color=Color.NC != "")) - print("") - - # Step 6: decide - if not diff.has_changes(): - print(f"{Color.GREEN}Nothing to apply.{Color.NC}") - return - - if dry_run: - print(f"{Color.CYAN}Dry run — no changes applied.{Color.NC}") - return - - if not yes: - resp = input("Apply these changes? [Y/n] ").strip().lower() - if resp not in ("", "y", "yes"): - print("Aborted.") - return - - # Step 7: reinstall (use existing install flow — it also writes new state.json) - # Use the determined scope and mode from the analysis above - from ..commands.install import install - local = (scope == "local") - copy = (mode == "copy") - install(local=local, copy=copy) \ No newline at end of file diff --git a/agent_notes/commands/wizard.py b/agent_notes/commands/wizard.py index ba633eb..f998648 100644 --- a/agent_notes/commands/wizard.py +++ b/agent_notes/commands/wizard.py @@ -8,6 +8,7 @@ from ._install_helpers import ( count_agents, count_global, count_skills ) +from ..services.counts import count_rules_total as _count_rules_total from ..services.fs import place_file, place_dir_contents from ..services.ui import ( _can_interactive, _safe_input, _checkbox_select, _radio_select, @@ -79,22 +80,7 @@ def _get_skill_groups() -> Dict[str, List[str]]: def _count_rules() -> int: """Count rule files.""" - # For testing, allow bypassing the registry - import os - if os.environ.get('_WIZARD_TEST_MODE'): - if not DIST_RULES_DIR.exists(): - return 0 - return len(list(DIST_RULES_DIR.glob("*.md"))) - else: - try: - from ..registries import default_rule_registry - registry = default_rule_registry() - return len(registry.all()) - except Exception: - # Fallback to old behavior if registry fails - if not DIST_RULES_DIR.exists(): - return 0 - return len(list(DIST_RULES_DIR.glob("*.md"))) + return _count_rules_total() def _select_cli(step: int = 0, total: int = 0, version: str = '') -> Set[str]: @@ -629,7 +615,27 @@ def _interactive_install() -> None: print(f"{Color.RED}Build failed: {e}{Color.NC}") return - # Execute installation + _execute_install( + clis=clis, + scope=scope, + copy_mode=copy_mode, + selected_skills=selected_skills, + role_models=role_models, + memory_backend=memory_backend, + memory_path=memory_path, + ) + + +def _execute_install( + clis: Set[str], + scope: str, + copy_mode: bool, + selected_skills: List[str], + role_models: Dict[str, Dict[str, str]], + memory_backend: str, + memory_path: str, +) -> None: + """Run all installation steps after parameters have been collected and the build is done.""" print(f"\nInstalling ({scope}, {'copy' if copy_mode else 'symlink'}) ...\n") from ..services import fs as _fs diff --git a/agent_notes/config.py b/agent_notes/config.py index b0ae85f..b733b6d 100644 --- a/agent_notes/config.py +++ b/agent_notes/config.py @@ -91,56 +91,46 @@ def global_output_path(backend) -> Optional[Path]: return dist_dir_for(backend) / backend.layout["config"] -# === Backward compatibility - lazy evaluation of old constants === -def _lazy_backend_attr(backend_name: str, attr_func): - """Helper for creating lazy attributes that depend on registry. - - Note: at initial config-module import time, ``agent_notes.cli_backend`` is - NOT safe to import (circular). In that case we fall back to hardcoded - values that match the shipped YAMLs. Later callers who import these - constants after initial module load get the correct values via this same - function (the registry is then importable). - """ - def get_attr(): - try: - from .registries.cli_registry import default_registry - backend = default_registry().get(backend_name) - return attr_func(backend) - except (ImportError, KeyError): - # Fallback to hardcoded values if registry not available (e.g. circular - # import at initial module-load time). These match the shipped YAMLs. - fallbacks = { - 'claude': {'home': Path.home() / ".claude", 'template': "global-claude.md", 'dist': "claude"}, - 'opencode': {'home': Path.home() / ".config" / "opencode", 'template': "global-opencode.md", 'dist': "opencode"}, - 'copilot': {'home': Path.home() / ".github", 'template': "global-copilot.md", 'dist': "github"}, - } - fb = fallbacks.get(backend_name, {}) - # Match by the attr_func's role — we encode intent via __name__ for named - # functions, or by returning the generic dist/home/template via the lambdas - # below (see DIST_*/GLOBAL_*/*_HOME assignments). - name = attr_func.__name__ - if name == 'global_home' or name == '_fb_home': - return fb.get('home') - elif name == 'template_path' or name == '_fb_template': - return DATA_DIR / fb.get('template', '') - elif name == 'dist_dir' or name == '_fb_dist': - return DIST_DIR / fb.get('dist', backend_name) - return get_attr - - -# Create backward-compatibility constants. We give the lambdas named stand-ins -# so the fallback branch in _lazy_backend_attr can classify them correctly even -# when the registry isn't importable yet (circular import during config init). -def _fb_home(b): return b.global_home -def _fb_dist(b): return dist_dir_for(b) -def _fb_template(b): return global_template_path(b) - -CLAUDE_HOME = _lazy_backend_attr('claude', _fb_home)() -OPENCODE_HOME = _lazy_backend_attr('opencode', _fb_home)() -GITHUB_HOME = _lazy_backend_attr('copilot', _fb_home)() -DIST_CLAUDE_DIR = _lazy_backend_attr('claude', _fb_dist)() -DIST_OPENCODE_DIR = _lazy_backend_attr('opencode', _fb_dist)() -DIST_GITHUB_DIR = _lazy_backend_attr('copilot', _fb_dist)() -GLOBAL_CLAUDE_MD = _lazy_backend_attr('claude', _fb_template)() -GLOBAL_OPENCODE_MD = _lazy_backend_attr('opencode', _fb_template)() -GLOBAL_COPILOT_MD = _lazy_backend_attr('copilot', _fb_template)() \ No newline at end of file +# === Backward compatibility constants === +# Hardcoded fallbacks match the shipped YAMLs and protect against circular +# imports if this module is ever imported very early in the load sequence. +_FALLBACKS = { + 'claude': {'home': Path.home() / ".claude", 'template': "global-claude.md", 'dist': "claude"}, + 'opencode': {'home': Path.home() / ".config" / "opencode", 'template': "global-opencode.md", 'dist': "opencode"}, + 'copilot': {'home': Path.home() / ".github", 'template': "global-copilot.md", 'dist': "github"}, +} + + +def _backend_home(name: str) -> Path: + try: + from .registries.cli_registry import default_registry + return default_registry().get(name).global_home + except (ImportError, KeyError): + return _FALLBACKS[name]['home'] + + +def _backend_dist(name: str) -> Path: + try: + from .registries.cli_registry import default_registry + return dist_dir_for(default_registry().get(name)) + except (ImportError, KeyError): + return DIST_DIR / _FALLBACKS[name]['dist'] + + +def _backend_template(name: str) -> Path: + try: + from .registries.cli_registry import default_registry + return global_template_path(default_registry().get(name)) + except (ImportError, KeyError): + return DATA_DIR / _FALLBACKS[name]['template'] + + +CLAUDE_HOME = _backend_home('claude') +OPENCODE_HOME = _backend_home('opencode') +GITHUB_HOME = _backend_home('copilot') +DIST_CLAUDE_DIR = _backend_dist('claude') +DIST_OPENCODE_DIR = _backend_dist('opencode') +DIST_GITHUB_DIR = _backend_dist('copilot') +GLOBAL_CLAUDE_MD = _backend_template('claude') +GLOBAL_OPENCODE_MD = _backend_template('opencode') +GLOBAL_COPILOT_MD = _backend_template('copilot') diff --git a/agent_notes/data/hooks/session-context.md.tpl b/agent_notes/data/hooks/session-context.md.tpl index 512c30e..a2e6b74 100644 --- a/agent_notes/data/hooks/session-context.md.tpl +++ b/agent_notes/data/hooks/session-context.md.tpl @@ -16,4 +16,4 @@ You have a specialized agent team installed. Delegate work to them — do not tr - Docs → tech-writer - Infrastructure → devops -**Slash commands:** /plan /review /debug /brainstorm +{{skills_catalog}} diff --git a/agent_notes/doctor_checks.py b/agent_notes/doctor_checks.py index 3965bd3..c26b1b0 100644 --- a/agent_notes/doctor_checks.py +++ b/agent_notes/doctor_checks.py @@ -1,6 +1,14 @@ """Scoped health checks for agent-notes — only touches files we own.""" from __future__ import annotations + +__all__ = [ + "expected_paths_for_install", + "check_missing", + "check_broken", + "check_drift", + "check_stale", +] from pathlib import Path from typing import Optional diff --git a/agent_notes/domain/state.py b/agent_notes/domain/state.py index 86249f2..7c54359 100644 --- a/agent_notes/domain/state.py +++ b/agent_notes/domain/state.py @@ -33,6 +33,7 @@ class ScopeState: installed_at: str = "" updated_at: str = "" mode: str = "symlink" + installed_version: str = "" clis: dict[str, BackendState] = field(default_factory=dict) diff --git a/agent_notes/scripts/cost_report.py b/agent_notes/scripts/cost_report.py index b3ecac7..964a5cb 100644 --- a/agent_notes/scripts/cost_report.py +++ b/agent_notes/scripts/cost_report.py @@ -5,6 +5,7 @@ from pathlib import Path from . import _claude_backend, _opencode_backend +from ._opencode_backend import DB as _OPENCODE_DB def _opencode_active() -> bool: @@ -21,8 +22,7 @@ def _by_recency(since: float | None = None, session_id: str | None = None) -> in if jsonls: claude_mtime = max(f.stat().st_mtime for f in jsonls) - opencode_db = Path.home() / ".local" / "share" / "opencode" / "opencode.db" - opencode_mtime = opencode_db.stat().st_mtime if opencode_db.exists() else 0.0 + opencode_mtime = _OPENCODE_DB.stat().st_mtime if _OPENCODE_DB.exists() else 0.0 if claude_mtime == 0.0 and opencode_mtime == 0.0: print("No session data found (no Claude Code transcripts or OpenCode database).") diff --git a/agent_notes/services/counts.py b/agent_notes/services/counts.py new file mode 100644 index 0000000..112409d --- /dev/null +++ b/agent_notes/services/counts.py @@ -0,0 +1,67 @@ +"""Shared counting helpers for installed components.""" + +import os + + +def count_rules_total() -> int: + """Count total available rule files (for wizard display). + + Uses the rule registry when available; falls back to counting .md files + directly from the dist rules directory. + """ + from ..config import DIST_RULES_DIR + + if os.environ.get('_WIZARD_TEST_MODE'): + if not DIST_RULES_DIR.exists(): + return 0 + return len(list(DIST_RULES_DIR.glob("*.md"))) + + try: + from ..registries import default_rule_registry + registry = default_rule_registry() + return len(registry.all()) + except Exception: + if not DIST_RULES_DIR.exists(): + return 0 + return len(list(DIST_RULES_DIR.glob("*.md"))) + + +def count_skills(backend, scope: str) -> tuple: + """Count (installed, expected) skills for a CLI backend. Excludes broken symlinks.""" + from . import installer + from ..config import DIST_SKILLS_DIR + + if not backend.supports("skills"): + return 0, 0 + + skills_dir = installer.target_dir_for(backend, "skills", scope) + if skills_dir and skills_dir.exists(): + installed = len([d for d in skills_dir.iterdir() if d.is_dir() and d.exists()]) + else: + installed = 0 + + expected = ( + len([d for d in DIST_SKILLS_DIR.iterdir() if d.is_dir()]) + if DIST_SKILLS_DIR and DIST_SKILLS_DIR.exists() + else 0 + ) + return installed, expected + + +def count_rules(backend, scope: str) -> tuple: + """Count (installed, expected) rules for a CLI backend.""" + from . import installer + from ..config import DIST_RULES_DIR + + if not backend.supports("rules"): + return 0, 0 + + rules_dir = installer.target_dir_for(backend, "rules", scope) + installed = len(list(rules_dir.glob("*.md"))) if rules_dir and rules_dir.exists() else 0 + + expected = ( + len(list(DIST_RULES_DIR.glob("*.md"))) + if DIST_RULES_DIR and DIST_RULES_DIR.exists() + else 0 + ) + return installed, expected diff --git a/agent_notes/services/diagnostics/_checks.py b/agent_notes/services/diagnostics/_checks.py index a2b9eb1..b2962f1 100644 --- a/agent_notes/services/diagnostics/_checks.py +++ b/agent_notes/services/diagnostics/_checks.py @@ -7,20 +7,30 @@ from ...domain.diagnostics import Issue, FixAction -def check_stale_files(scope: str, issues: List[Issue], fix_actions: List[FixAction]): - """Check for installed files without matching source - DELEGATED to doctor_checks.""" - # This function is kept for backwards compatibility but delegates to the new module - from ...cli_backend import load_registry - from ... import install_state, doctor_checks +def _load_scope_state(scope: str): + """Load registry and scope state for a given scope. + + Returns (registry, scope_state) where scope_state may be None if no + state file exists. + """ + from ...registries.cli_registry import load_registry + from ... import install_state from ...state import get_scope registry = load_registry() state = install_state.load_current_state() if state is None: - scope_state = None - else: - project_path = Path.cwd() if scope == "local" else None - scope_state = get_scope(state, scope, project_path) + return registry, None + project_path = Path.cwd() if scope == "local" else None + scope_state = get_scope(state, scope, project_path) + return registry, scope_state + + +def check_stale_files(scope: str, issues: List[Issue], fix_actions: List[FixAction]): + """Check for installed files without matching source - DELEGATED to doctor_checks.""" + from ... import doctor_checks + + registry, scope_state = _load_scope_state(scope) doctor_checks.check_stale(scope, scope_state, registry, issues, fix_actions) @@ -30,7 +40,7 @@ def _find_dist_source(symlink: Path, scope: str) -> Optional[Path]: Iterates all registered backends; returns first dist source whose component and filename match the given symlink. """ - from ...cli_backend import load_registry + from ...registries.cli_registry import load_registry from ... import installer registry = load_registry() @@ -75,35 +85,17 @@ def _get_dist_skills_dir(): def check_broken_symlinks(scope: str, issues: List[Issue], fix_actions: List[FixAction]): """Check for symlinks with non-existent targets - DELEGATED to doctor_checks.""" - # This function is kept for backwards compatibility but delegates to the new module - from ...cli_backend import load_registry - from ... import install_state, doctor_checks - from ...state import get_scope + from ... import doctor_checks - registry = load_registry() - state = install_state.load_current_state() - if state is None: - scope_state = None - else: - project_path = Path.cwd() if scope == "local" else None - scope_state = get_scope(state, scope, project_path) + registry, scope_state = _load_scope_state(scope) doctor_checks.check_broken(scope, registry, issues, fix_actions, scope_state) def check_shadowed_files(scope: str, issues: List[Issue], fix_actions: List[FixAction]): """Check for regular files where symlinks are expected - TARGETED check only.""" - from ...cli_backend import load_registry - from ... import install_state, doctor_checks - from ...state import get_scope + from ... import doctor_checks - # Get expected paths and check each one individually - registry = load_registry() - state = install_state.load_current_state() - if state is None: - scope_state = None - else: - project_path = Path.cwd() if scope == "local" else None - scope_state = get_scope(state, scope, project_path) + registry, scope_state = _load_scope_state(scope) # Only check paths we know should exist for src, dst, backend_name, component in doctor_checks.expected_paths_for_install(registry, scope): @@ -119,13 +111,14 @@ def check_shadowed_files(scope: str, issues: List[Issue], fix_actions: List[FixA def check_missing_files(scope: str, issues: List[Issue], fix_actions: List[FixAction]): """Check for source files that aren't installed - DELEGATED to doctor_checks.""" - # This function is kept for backwards compatibility but delegates to the new module - from ...cli_backend import load_registry - from ... import doctor_checks, install_state + from ... import doctor_checks + from ...registries.cli_registry import load_registry + from ... import install_state from ...state import get_scope registry = load_registry() # Pass scope state so opted-out backends aren't flagged as "missing". + # Use a try/except here since local scope resolution can raise ValueError/KeyError. state = install_state.load_current_state() scope_state = None if state is not None: @@ -144,18 +137,9 @@ def check_missing_files(scope: str, issues: List[Issue], fix_actions: List[FixAc def check_content_drift(scope: str, issues: List[Issue], fix_actions: List[FixAction]): """Check for copied files that differ from source - DELEGATED to doctor_checks.""" - # This function is kept for backwards compatibility but delegates to the new module - from ...cli_backend import load_registry - from ... import install_state, doctor_checks - from ...state import get_scope + from ... import doctor_checks - registry = load_registry() - state = install_state.load_current_state() - if state is None: - scope_state = None - else: - project_path = Path.cwd() if scope == "local" else None - scope_state = get_scope(state, scope, project_path) + registry, scope_state = _load_scope_state(scope) doctor_checks.check_drift(scope, registry, issues, fix_actions, scope_state) @@ -167,7 +151,7 @@ def check_build_freshness(issues: List[Issue], fix_actions: List[FixAction]): # Check agents.yaml vs generated agents if agents_yaml.exists(): source_time = agents_yaml.stat().st_mtime - from ...cli_backend import load_registry + from ...registries.cli_registry import load_registry from ...config import dist_dir_for registry = load_registry() @@ -184,7 +168,7 @@ def check_build_freshness(issues: List[Issue], fix_actions: List[FixAction]): # Check individual source agents source_agents_dir = AGENTS_DIR if source_agents_dir.exists(): - from ...cli_backend import load_registry + from ...registries.cli_registry import load_registry from ...config import dist_dir_for registry = load_registry() @@ -202,7 +186,7 @@ def check_build_freshness(issues: List[Issue], fix_actions: List[FixAction]): fix_actions.append(FixAction("BUILD", str(gen_file), "regenerate from source")) # Check global source files - from ...cli_backend import load_registry + from ...registries.cli_registry import load_registry from ...config import global_template_path, global_output_path registry = load_registry() diff --git a/agent_notes/services/diagnostics/_display.py b/agent_notes/services/diagnostics/_display.py index 788a557..502b8d6 100644 --- a/agent_notes/services/diagnostics/_display.py +++ b/agent_notes/services/diagnostics/_display.py @@ -1,9 +1,16 @@ """Display and summary functions for agent-notes diagnostics.""" +__all__ = [ + "count_stale", + "print_summary", + "print_issues", +] + from pathlib import Path from typing import List, Dict from ...domain.diagnostics import Issue +from ..counts import count_skills as _count_skills_impl, count_rules as _count_rules_impl def _cli_base_dir(backend, scope: str) -> Path: @@ -34,50 +41,12 @@ def _count_agents(backend, scope: str) -> tuple: def _count_skills(backend, scope: str) -> tuple: """Count (installed, expected) skills for a CLI backend. Excludes broken symlinks.""" - from ... import installer - - # Helper to get DIST_SKILLS_DIR - def _get_dist_skills_dir(): - from ...config import DIST_SKILLS_DIR - return DIST_SKILLS_DIR - - if not backend.supports("skills"): - return 0, 0 - - # Count installed - skills_dir = installer.target_dir_for(backend, "skills", scope) - if skills_dir and skills_dir.exists(): - installed = len([d for d in skills_dir.iterdir() if d.is_dir() and d.exists()]) - else: - installed = 0 - - # Count expected (universal skills) - dist_skills_dir = _get_dist_skills_dir() - expected = len([d for d in dist_skills_dir.iterdir() if d.is_dir()]) if dist_skills_dir and dist_skills_dir.exists() else 0 - return installed, expected - + return _count_skills_impl(backend, scope) def _count_rules(backend, scope: str) -> tuple: """Count (installed, expected) rules for a CLI backend.""" - from ... import installer - - # Helper to get DIST_RULES_DIR - def _get_dist_rules_dir(): - from ...config import DIST_RULES_DIR - return DIST_RULES_DIR - - if not backend.supports("rules"): - return 0, 0 - - # Count installed - rules_dir = installer.target_dir_for(backend, "rules", scope) - installed = len(list(rules_dir.glob("*.md"))) if rules_dir and rules_dir.exists() else 0 - - # Count expected - dist_rules_dir = _get_dist_rules_dir() - expected = len(list(dist_rules_dir.glob("*.md"))) if dist_rules_dir and dist_rules_dir.exists() else 0 - return installed, expected + return _count_rules_impl(backend, scope) def _check_config(backend, scope: str) -> tuple: @@ -101,9 +70,9 @@ def _check_config(backend, scope: str) -> tuple: def _check_role_models(state): """Display role→model assignments and check compatibility.""" - from ...model_registry import load_model_registry + from ...registries.model_registry import load_model_registry from ...registries.cli_registry import load_registry - from ...role_registry import load_role_registry + from ...registries.role_registry import load_role_registry from ...config import Color model_registry = load_model_registry() diff --git a/agent_notes/services/install_state_builder.py b/agent_notes/services/install_state_builder.py index 79acf40..d8a7770 100644 --- a/agent_notes/services/install_state_builder.py +++ b/agent_notes/services/install_state_builder.py @@ -171,10 +171,12 @@ def build_install_state( clis[backend.name] = backend_state # Create the new scope state + from ..config import get_version new_scope_state = ScopeState( installed_at=timestamp, updated_at=timestamp, mode=mode, + installed_version=get_version(), clis=clis, ) diff --git a/agent_notes/services/installer.py b/agent_notes/services/installer.py index 8a77014..b1f7ab1 100644 --- a/agent_notes/services/installer.py +++ b/agent_notes/services/installer.py @@ -393,6 +393,7 @@ def _install_session_hook(backend, scope: str) -> None: """Install the SessionStart hook and write the context file for Claude Code.""" from .settings_writer import install_hook, install_allow_entry, remove_allow_entry from .session_context import write_context + from ..registries.skill_registry import default_skill_registry from .. import config settings_path, context_file, hook_command = _session_hook_paths(backend, scope) @@ -403,9 +404,11 @@ def _install_session_hook(backend, scope: str) -> None: if agents_dist.exists(): agents = sorted(p.stem for p in agents_dist.glob("*.md")) + skills = default_skill_registry().all() + version = config.get_version() print(f"Installing Claude Code SessionStart hook ...") - write_context(context_file, agents, version) + write_context(context_file, agents, version, skills) install_hook(settings_path, "SessionStart", hook_command) # Remove old standalone entry if present (backward compat cleanup) remove_allow_entry(settings_path, "Bash(cost-report)") diff --git a/agent_notes/services/session_context.py b/agent_notes/services/session_context.py index 2caa754..3d4e513 100644 --- a/agent_notes/services/session_context.py +++ b/agent_notes/services/session_context.py @@ -1,23 +1,36 @@ """Generate the session context file injected by the SessionStart hook.""" from __future__ import annotations from pathlib import Path +from typing import Any def _template_path() -> Path: return Path(__file__).parent.parent / "data" / "hooks" / "session-context.md.tpl" -def render_context(agents: list[str], version: str) -> str: +def _render_skills_catalog(skills: list[Any]) -> str: + if not skills: + return "" + sorted_skills = sorted(skills, key=lambda s: s.name) + lines = ["**Skills** (invoke with /skill-name):"] + for skill in sorted_skills: + lines.append(f"- /{skill.name} — {skill.description}") + return "\n".join(lines) + + +def render_context(agents: list[str], version: str, skills: list[Any] | None = None) -> str: tpl = _template_path().read_text() if agents: agents_list = "\n".join(f"- {name}" for name in sorted(agents)) else: agents_list = "- (see ~/.claude/agents/ for installed agents)" + skills_catalog = _render_skills_catalog(skills) if skills else "" return (tpl .replace("{{version}}", version) - .replace("{{agents_list}}", agents_list)) + .replace("{{agents_list}}", agents_list) + .replace("{{skills_catalog}}", skills_catalog)) -def write_context(dest: Path, agents: list[str], version: str) -> None: +def write_context(dest: Path, agents: list[str], version: str, skills: list[Any] | None = None) -> None: dest.parent.mkdir(parents=True, exist_ok=True) - dest.write_text(render_context(agents, version)) + dest.write_text(render_context(agents, version, skills)) diff --git a/agent_notes/services/state_store.py b/agent_notes/services/state_store.py index 287f062..07a9989 100644 --- a/agent_notes/services/state_store.py +++ b/agent_notes/services/state_store.py @@ -154,6 +154,7 @@ def _scope_to_dict(s: ScopeState) -> dict: "installed_at": s.installed_at, "updated_at": s.updated_at, "mode": s.mode, + "installed_version": s.installed_version, "clis": {name: _backend_to_dict(bs) for name, bs in s.clis.items()}, } @@ -200,6 +201,7 @@ def _scope_from_dict(data: dict) -> ScopeState: installed_at=data.get("installed_at", ""), updated_at=data.get("updated_at", ""), mode=data.get("mode", "symlink"), + installed_version=data.get("installed_version", ""), clis=clis, ) diff --git a/tests/functional/commands/test_doctor_command.py b/tests/functional/commands/test_doctor_command.py index 2f5a5f5..c7dafe7 100644 --- a/tests/functional/commands/test_doctor_command.py +++ b/tests/functional/commands/test_doctor_command.py @@ -7,6 +7,8 @@ import agent_notes.config as config from agent_notes.domain.diagnostics import Issue, FixAction +from agent_notes.domain.state import ScopeState, State, MemoryConfig +from agent_notes.services.state_store import save_state, load_state, _scope_to_dict # ── Helpers ──────────────────────────────────────────────────────────────────── @@ -148,3 +150,255 @@ def fake_broken_symlink(scope, issues, fix_actions): mock_do_fix.assert_called_once() issues_arg = mock_do_fix.call_args[0][0] assert len(issues_arg) > 0, "do_fix should receive the list of issues" + + +# ── check_version_drift tests ───────────────────────────────────────────────── + +def _write_state_with_version(sf: Path, installed_version: str) -> None: + """Write a minimal state.json with the given installed_version in the global scope.""" + scope_dict = { + "installed_at": "2025-01-01T00:00:00Z", + "updated_at": "2025-01-01T00:00:00Z", + "mode": "symlink", + "installed_version": installed_version, + "clis": {"claude": {"role_models": {}, "installed": {}}}, + } + data = { + "source_path": "/tmp/repo", + "source_commit": "abc123", + "global": scope_dict, + "local": {}, + "memory": {"backend": "local", "path": ""}, + } + sf.parent.mkdir(parents=True, exist_ok=True) + sf.write_text(json.dumps(data)) + + +def _setup_xdg(tmp_path, monkeypatch) -> Path: + """Point XDG_CONFIG_HOME to tmp_path and return the state file path.""" + xdg = tmp_path / "config" + xdg.mkdir() + monkeypatch.setenv("XDG_CONFIG_HOME", str(xdg)) + return xdg / "agent-notes" / "state.json" + + +class TestCheckVersionDriftVersionMatches: + def test_no_issues_when_installed_version_matches_current(self, tmp_path, monkeypatch): + sf = _setup_xdg(tmp_path, monkeypatch) + _write_state_with_version(sf, "1.2.3") + + from agent_notes.commands.doctor import check_version_drift + with patch("agent_notes.commands.doctor.check_version_drift.__wrapped__", None, create=True), \ + patch("agent_notes.config.get_version", return_value="1.2.3"): + # Re-import to get fresh reference with patched get_version + import agent_notes.config as _cfg + monkeypatch.setattr(_cfg, "get_version", lambda: "1.2.3") + + issues: list = [] + fix_actions: list = [] + check_version_drift("global", issues, fix_actions) + + assert issues == [], "no issues expected when versions match" + assert fix_actions == [], "no fix actions expected when versions match" + + +class TestCheckVersionDriftVersionDiffers: + def test_reports_drift_issue_when_versions_differ(self, tmp_path, monkeypatch): + sf = _setup_xdg(tmp_path, monkeypatch) + _write_state_with_version(sf, "1.0.0") + + import agent_notes.config as _cfg + monkeypatch.setattr(_cfg, "get_version", lambda: "2.0.0") + + from agent_notes.commands.doctor import check_version_drift + issues: list = [] + fix_actions: list = [] + check_version_drift("global", issues, fix_actions) + + assert len(issues) == 1, "exactly one issue expected for version drift" + issue = issues[0] + assert issue.type == "version_drift" + assert "1.0.0" in issue.message + assert "2.0.0" in issue.message + + def test_adds_trigger_install_fix_action_when_versions_differ(self, tmp_path, monkeypatch): + sf = _setup_xdg(tmp_path, monkeypatch) + _write_state_with_version(sf, "1.0.0") + + import agent_notes.config as _cfg + monkeypatch.setattr(_cfg, "get_version", lambda: "2.0.0") + + from agent_notes.commands.doctor import check_version_drift + issues: list = [] + fix_actions: list = [] + check_version_drift("global", issues, fix_actions) + + assert len(fix_actions) == 1, "exactly one fix action expected" + assert fix_actions[0].action == "_TRIGGER_INSTALL" + + +class TestCheckVersionDriftEmptyInstalledVersion: + def test_no_issues_when_installed_version_is_empty(self, tmp_path, monkeypatch): + """Old state.json files from before version tracking have empty installed_version. + The function should handle this gracefully — not crash, and not report drift.""" + sf = _setup_xdg(tmp_path, monkeypatch) + _write_state_with_version(sf, "") # old-style state: no version recorded + + import agent_notes.config as _cfg + monkeypatch.setattr(_cfg, "get_version", lambda: "2.0.0") + + from agent_notes.commands.doctor import check_version_drift + issues: list = [] + fix_actions: list = [] + check_version_drift("global", issues, fix_actions) + + # Per implementation: empty installed_version → return early (skip check) + assert issues == [], "empty installed_version should not produce a version_drift issue" + assert fix_actions == [], "empty installed_version should not produce a fix action" + + def test_no_crash_when_state_has_no_version_field(self, tmp_path, monkeypatch): + """State dict with no 'installed_version' key at all (not even empty string).""" + xdg = tmp_path / "config" + xdg.mkdir() + monkeypatch.setenv("XDG_CONFIG_HOME", str(xdg)) + sf = xdg / "agent-notes" / "state.json" + + # Write state without installed_version key + scope_dict = { + "installed_at": "2025-01-01T00:00:00Z", + "updated_at": "2025-01-01T00:00:00Z", + "mode": "symlink", + # no "installed_version" key + "clis": {"claude": {"role_models": {}, "installed": {}}}, + } + data = { + "source_path": "/tmp/repo", + "source_commit": "abc123", + "global": scope_dict, + "local": {}, + "memory": {"backend": "local", "path": ""}, + } + sf.parent.mkdir(parents=True, exist_ok=True) + sf.write_text(json.dumps(data)) + + import agent_notes.config as _cfg + monkeypatch.setattr(_cfg, "get_version", lambda: "2.0.0") + + from agent_notes.commands.doctor import check_version_drift + issues: list = [] + fix_actions: list = [] + # Must not raise + check_version_drift("global", issues, fix_actions) + + assert issues == [], "missing installed_version key should not produce a drift issue" + + +class TestCheckVersionDriftNoState: + def test_no_issues_when_state_file_absent(self, tmp_path, monkeypatch): + """When no state.json exists at all, check_version_drift should return silently.""" + xdg = tmp_path / "config" + xdg.mkdir() + monkeypatch.setenv("XDG_CONFIG_HOME", str(xdg)) + # deliberately no state.json written + + import agent_notes.config as _cfg + monkeypatch.setattr(_cfg, "get_version", lambda: "2.0.0") + + from agent_notes.commands.doctor import check_version_drift + issues: list = [] + fix_actions: list = [] + check_version_drift("global", issues, fix_actions) + + assert issues == [] + assert fix_actions == [] + + +class TestInstalledVersionSerializationRoundTrip: + def test_installed_version_persists_through_save_and_load(self, tmp_path, monkeypatch): + """installed_version survives a save_state / load_state round-trip.""" + xdg = tmp_path / "config" + xdg.mkdir() + monkeypatch.setenv("XDG_CONFIG_HOME", str(xdg)) + + from agent_notes.services.state_store import save_state, load_state + from agent_notes.domain.state import State, ScopeState, MemoryConfig + + scope = ScopeState( + installed_at="2025-01-01T00:00:00Z", + updated_at="2025-01-01T00:00:00Z", + mode="symlink", + installed_version="1.9.9", + clis={}, + ) + state = State( + source_path="/tmp/repo", + source_commit="abc123", + global_install=scope, + local_installs={}, + memory=MemoryConfig(backend="local", path=""), + ) + save_state(state) + + loaded = load_state() + assert loaded is not None + assert loaded.global_install is not None + assert loaded.global_install.installed_version == "1.9.9" + + def test_scope_to_dict_includes_installed_version(self): + """_scope_to_dict serializes installed_version into the output dict.""" + from agent_notes.services.state_store import _scope_to_dict + from agent_notes.domain.state import ScopeState + + scope = ScopeState( + installed_at="2025-01-01T00:00:00Z", + updated_at="2025-01-01T00:00:00Z", + mode="symlink", + installed_version="3.0.1", + clis={}, + ) + d = _scope_to_dict(scope) + assert "installed_version" in d + assert d["installed_version"] == "3.0.1" + + def test_empty_installed_version_round_trips_as_empty_string(self, tmp_path, monkeypatch): + """An empty installed_version serializes and deserializes as empty string, not None.""" + xdg = tmp_path / "config" + xdg.mkdir() + monkeypatch.setenv("XDG_CONFIG_HOME", str(xdg)) + + from agent_notes.services.state_store import save_state, load_state + from agent_notes.domain.state import State, ScopeState, MemoryConfig + + scope = ScopeState( + installed_at="2025-01-01T00:00:00Z", + updated_at="2025-01-01T00:00:00Z", + mode="symlink", + installed_version="", + clis={}, + ) + state = State( + source_path="/tmp/repo", + source_commit="abc123", + global_install=scope, + local_installs={}, + memory=MemoryConfig(backend="local", path=""), + ) + save_state(state) + + loaded = load_state() + assert loaded is not None + assert loaded.global_install is not None + # Must come back as empty string, not None or missing + assert loaded.global_install.installed_version == "" + + +class TestUpdateCommandRemoved: + def test_update_command_module_does_not_exist(self): + """The `update` command should have been removed from the commands package.""" + import importlib + import importlib.util + spec = importlib.util.find_spec("agent_notes.commands.update") + assert spec is None, ( + "agent_notes.commands.update still exists — " + "the update command should have been removed" + ) diff --git a/tests/functional/commands/test_update_command.py b/tests/functional/commands/test_update_command.py deleted file mode 100644 index fae3a55..0000000 --- a/tests/functional/commands/test_update_command.py +++ /dev/null @@ -1,110 +0,0 @@ -"""Functional tests for the update command.""" - -import json -import pytest -from pathlib import Path -from unittest.mock import patch, MagicMock - -import agent_notes.config as config - - -# ── Helpers ──────────────────────────────────────────────────────────────────── - -# build() is lazily imported inside update(), so patch at definition site. -_PATCH_BUILD = "agent_notes.commands.build.build" -# install() is lazily imported inside update(), so patch at definition site. -_PATCH_INSTALL = "agent_notes.commands.install.install" - - -def _write_minimal_state(sf: Path) -> None: - data = { - "source_path": "/tmp/repo", - "source_commit": "abc123", - "global": { - "installed_at": "2025-01-01T00:00:00Z", - "updated_at": "2025-01-01T00:00:00Z", - "mode": "symlink", - "clis": {"claude": {"role_models": {}, "installed": {}}}, - }, - "local": {}, - "memory": {"backend": "local", "path": ""}, - } - sf.parent.mkdir(parents=True, exist_ok=True) - sf.write_text(json.dumps(data)) - - -def _setup(tmp_path, monkeypatch): - """Redirect state.json and dist to tmp; write minimal state.""" - xdg = tmp_path / "config" - xdg.mkdir() - monkeypatch.setenv("XDG_CONFIG_HOME", str(xdg)) - sf = xdg / "agent-notes" / "state.json" - _write_minimal_state(sf) - monkeypatch.setattr(config, "DIST_DIR", tmp_path / "dist") - monkeypatch.setattr(config, "DIST_SKILLS_DIR", tmp_path / "dist" / "skills") - monkeypatch.setattr(config, "DIST_RULES_DIR", tmp_path / "dist" / "rules") - return sf - - -def _fake_diff(has_changes: bool = False): - d = MagicMock() - d.has_changes.return_value = has_changes - return d - - -# ── Tests ────────────────────────────────────────────────────────────────────── - -class TestUpdateCallsBuild: - def test_update_calls_build(self, tmp_path, monkeypatch): - _setup(tmp_path, monkeypatch) - mock_build = MagicMock() - fake_diff = _fake_diff(has_changes=False) - - with patch(_PATCH_BUILD, mock_build), \ - patch("agent_notes.services.diff.diff_states", return_value=fake_diff), \ - patch("agent_notes.services.diff.render_diff_report", return_value=""), \ - patch("agent_notes.services.diff.filter_diff", return_value=fake_diff), \ - patch("agent_notes.services.install_state_builder.git_head_short", return_value="abc"): - from agent_notes.commands.update import update - update(skip_pull=True) - - mock_build.assert_called_once() - - -class TestUpdateCallsInstallAfterBuild: - def test_update_calls_install_after_build(self, tmp_path, monkeypatch): - _setup(tmp_path, monkeypatch) - fake_diff = _fake_diff(has_changes=True) - mock_install = MagicMock() - - with patch(_PATCH_BUILD), \ - patch("agent_notes.services.diff.diff_states", return_value=fake_diff), \ - patch("agent_notes.services.diff.render_diff_report", return_value=""), \ - patch("agent_notes.services.diff.filter_diff", return_value=fake_diff), \ - patch("agent_notes.services.install_state_builder.git_head_short", return_value="abc"), \ - patch(_PATCH_INSTALL, mock_install), \ - patch("builtins.input", return_value="y"): - from agent_notes.commands.update import update - update(skip_pull=True) - - mock_install.assert_called_once() - - -class TestUpdateDryRun: - def test_update_does_not_alter_state_when_dry_run(self, tmp_path, monkeypatch): - sf = _setup(tmp_path, monkeypatch) - original = sf.read_text() - fake_diff = _fake_diff(has_changes=True) - mock_install = MagicMock() - - with patch(_PATCH_BUILD), \ - patch("agent_notes.services.diff.diff_states", return_value=fake_diff), \ - patch("agent_notes.services.diff.render_diff_report", return_value=""), \ - patch("agent_notes.services.diff.filter_diff", return_value=fake_diff), \ - patch("agent_notes.services.install_state_builder.git_head_short", return_value="abc"), \ - patch(_PATCH_INSTALL, mock_install): - from agent_notes.commands.update import update - update(skip_pull=True, dry_run=True) - - mock_install.assert_not_called() - assert sf.read_text() == original, "dry-run must not modify state.json" diff --git a/tests/unit/services/test_session_context.py b/tests/unit/services/test_session_context.py new file mode 100644 index 0000000..dea3e62 --- /dev/null +++ b/tests/unit/services/test_session_context.py @@ -0,0 +1,132 @@ +"""Tests for agent_notes.services.session_context — _render_skills_catalog and render_context.""" +import pytest +from pathlib import Path + +from agent_notes.domain.skill import Skill +from agent_notes.services.session_context import _render_skills_catalog, render_context + + +def _skill(name: str, description: str = "", group: str | None = None) -> Skill: + """Build a minimal Skill fixture without touching the filesystem.""" + return Skill(name=name, path=Path("/fake") / name, description=description, group=group) + + +class TestRenderSkillsCatalog: + def test_empty_list_returns_empty_string(self): + assert _render_skills_catalog([]) == "" + + def test_none_equivalent_empty_handled_by_caller(self): + """render_context passes [] when skills is falsy; catalog itself only receives lists.""" + assert _render_skills_catalog([]) == "" + + def test_single_skill_renders_name_and_description(self): + result = _render_skills_catalog([_skill("brainstorming", "Explore multiple approaches")]) + assert "/brainstorming" in result + assert "Explore multiple approaches" in result + + def test_single_skill_uses_em_dash_separator(self): + result = _render_skills_catalog([_skill("brainstorming", "Explore multiple approaches")]) + assert "— Explore multiple approaches" in result + + def test_multiple_skills_sorted_alphabetically(self): + skills = [ + _skill("zebra", "Last alphabetically"), + _skill("alpha", "First alphabetically"), + _skill("middle", "In between"), + ] + result = _render_skills_catalog(skills) + alpha_pos = result.index("/alpha") + middle_pos = result.index("/middle") + zebra_pos = result.index("/zebra") + assert alpha_pos < middle_pos < zebra_pos + + def test_header_line_is_present(self): + result = _render_skills_catalog([_skill("foo", "bar")]) + assert "**Skills**" in result + assert "/skill-name" in result + + def test_skill_with_no_description_still_renders(self): + result = _render_skills_catalog([_skill("silent", description="")]) + assert "/silent" in result + + def test_each_skill_appears_on_its_own_line(self): + skills = [_skill("aaa", "desc a"), _skill("bbb", "desc b")] + lines = _render_skills_catalog(skills).splitlines() + skill_lines = [l for l in lines if l.startswith("- /")] + assert len(skill_lines) == 2 + + def test_skills_with_groups_are_still_rendered(self): + skills = [_skill("git", "Git workflow helper", group="scm")] + result = _render_skills_catalog(skills) + assert "/git" in result + assert "Git workflow helper" in result + + +class TestRenderContext: + def test_version_substituted(self): + result = render_context(agents=[], version="9.9.9") + assert "9.9.9" in result + assert "{{version}}" not in result + + def test_agents_list_substituted_with_provided_agents(self): + result = render_context(agents=["coder", "explorer"], version="1.0") + assert "- coder" in result + assert "- explorer" in result + assert "{{agents_list}}" not in result + + def test_empty_agents_uses_fallback_message(self): + result = render_context(agents=[], version="1.0") + assert "~/.claude/agents/" in result + assert "{{agents_list}}" not in result + + def test_agents_sorted_alphabetically(self): + result = render_context(agents=["zebra", "alpha"], version="1.0") + alpha_pos = result.index("- alpha") + zebra_pos = result.index("- zebra") + assert alpha_pos < zebra_pos + + def test_skills_none_removes_placeholder(self): + result = render_context(agents=[], version="1.0", skills=None) + assert "{{skills_catalog}}" not in result + + def test_skills_empty_list_removes_placeholder(self): + result = render_context(agents=[], version="1.0", skills=[]) + assert "{{skills_catalog}}" not in result + + def test_skills_empty_list_injects_no_catalog_text(self): + result = render_context(agents=[], version="1.0", skills=[]) + assert "**Skills**" not in result + + def test_skills_provided_injects_catalog(self): + skills = [_skill("brainstorming", "Explore multiple approaches")] + result = render_context(agents=[], version="1.0", skills=skills) + assert "**Skills**" in result + assert "/brainstorming" in result + assert "Explore multiple approaches" in result + + def test_skills_placeholder_replaced_when_skills_provided(self): + skills = [_skill("caveman", "Ultra-compressed communication")] + result = render_context(agents=[], version="1.0", skills=skills) + assert "{{skills_catalog}}" not in result + + def test_no_literal_placeholders_remain_with_full_render(self): + skills = [_skill("git", "Git helper"), _skill("brainstorming", "Explore ideas")] + result = render_context(agents=["coder", "reviewer"], version="2.0", skills=skills) + assert "{{version}}" not in result + assert "{{agents_list}}" not in result + assert "{{skills_catalog}}" not in result + + def test_full_render_contains_all_three_sections(self): + """version, agents, and skills all appear together in a full render.""" + skills = [_skill("caveman", "Compressed comms")] + result = render_context(agents=["coder"], version="3.1.4", skills=skills) + assert "3.1.4" in result + assert "- coder" in result + assert "/caveman" in result + assert "Compressed comms" in result + + def test_default_skills_param_is_none(self): + """render_context(agents, version) with no skills kwarg must not raise.""" + result = render_context(agents=["coder"], version="1.0") + assert "{{skills_catalog}}" not in result + assert "1.0" in result diff --git a/tests/unit/test_import_health.py b/tests/unit/test_import_health.py new file mode 100644 index 0000000..ee42923 --- /dev/null +++ b/tests/unit/test_import_health.py @@ -0,0 +1,260 @@ +"""Validate that all deferred imports in the diagnostics subsystem resolve correctly. + +Deferred imports (imports inside function bodies) only crash at runtime when the +function is actually called — not at module load time. This test suite uses AST +parsing to extract every deferred import statement from _checks.py, _display.py, +and _fix.py, converts relative imports to absolute module paths, and asserts each +one is importable without ModuleNotFoundError. +""" +import ast +import importlib +import textwrap +from pathlib import Path +from typing import List, Tuple + +import pytest + +# --------------------------------------------------------------------------- +# Helpers — AST extraction +# --------------------------------------------------------------------------- + +_DIAGNOSTICS_PKG = "agent_notes.services.diagnostics" + +# Relative depth from _checks.py / _display.py / _fix.py to the package root. +# Each file lives at: agent_notes/services/diagnostics/.py +# One level up → agent_notes/services/diagnostics (depth 1) +# Two levels up → agent_notes/services (depth 2) +# Three levels → agent_notes (depth 3) +# The source files use `from ... import` (level=3) to reach agent_notes.* +_MODULE_ROOT = "agent_notes" + + +def _anchor_package(filename: str) -> str: + """Return the dotted package that the source file belongs to. + + _checks.py → agent_notes.services.diagnostics + """ + # All three files live inside agent_notes/services/diagnostics/ + return _DIAGNOSTICS_PKG + + +def _resolve_relative_import(level: int, module_part: str, anchor: str) -> str: + """Convert a relative import (level, module_part) to an absolute module path. + + level=1 means 'current package'; level=3 means 'three packages up'. + anchor is the dotted package path of the file containing the import. + """ + parts = anchor.split(".") + # Walk 'level' steps up (level=1 → stay in current package, level=2 → one up, …) + up_count = level - 1 + base_parts = parts[: len(parts) - up_count] if up_count < len(parts) else parts[:1] + base = ".".join(base_parts) + if module_part: + return f"{base}.{module_part}" + return base + + +def _extract_deferred_imports(source_path: Path, anchor: str) -> List[Tuple[str, str]]: + """Parse *source_path* and return (module_path, repr_for_display) for every + import that appears inside a function or method body (i.e. deferred imports). + + Returns only the module-level part — not the names imported from it, since + we only need to verify the module itself is importable. + """ + source = source_path.read_text(encoding="utf-8") + tree = ast.parse(source, filename=str(source_path)) + + results: List[Tuple[str, str]] = [] + + for node in ast.walk(tree): + # We want imports that are NOT at module level — i.e. they appear inside + # a FunctionDef / AsyncFunctionDef body. ast.walk visits everything, so + # we need to check parent context. Instead, walk only the bodies of + # function-like nodes. + if not isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)): + continue + + for child in ast.walk(node): + if isinstance(child, ast.Import): + for alias in child.names: + results.append((alias.name, f"import {alias.name}")) + + elif isinstance(child, ast.ImportFrom): + if child.module is None: + # Bare `from . import something` — the module IS the package + module_path = _resolve_relative_import(child.level or 0, "", anchor) + else: + if child.level and child.level > 0: + module_path = _resolve_relative_import( + child.level, child.module, anchor + ) + else: + module_path = child.module + results.append( + (module_path, f"from {'.' * (child.level or 0)}{child.module or ''} import …") + ) + + # Deduplicate while preserving order + seen: set = set() + unique = [] + for mod, label in results: + if mod not in seen: + seen.add(mod) + unique.append((mod, label)) + return unique + + +# --------------------------------------------------------------------------- +# Collect imports from the three diagnostics source files +# --------------------------------------------------------------------------- + +_DIAG_DIR = ( + Path(__file__).resolve().parent.parent.parent + / "agent_notes" + / "services" + / "diagnostics" +) + +_SOURCE_FILES = [ + _DIAG_DIR / "_checks.py", + _DIAG_DIR / "_display.py", + _DIAG_DIR / "_fix.py", +] + +_ANCHOR = _anchor_package("_checks.py") # same anchor for all three + +_ALL_DEFERRED_IMPORTS: List[Tuple[str, str, str]] = [] +for _src_file in _SOURCE_FILES: + for _mod, _label in _extract_deferred_imports(_src_file, _ANCHOR): + _ALL_DEFERRED_IMPORTS.append((_mod, _label, _src_file.name)) + + +# --------------------------------------------------------------------------- +# Tests +# --------------------------------------------------------------------------- + + +class TestDeferredImportsResolve: + """Every deferred (function-body) import in the diagnostics subsystem must + resolve to an importable module without raising ModuleNotFoundError.""" + + @pytest.mark.parametrize( + "module_path,label,source_file", + _ALL_DEFERRED_IMPORTS, + ids=[ + f"{src}::{mod}" for mod, _label, src in _ALL_DEFERRED_IMPORTS + ], + ) + def test_deferred_import_is_importable( + self, module_path: str, label: str, source_file: str + ): + """Importing *module_path* must not raise ModuleNotFoundError. + + A ModuleNotFoundError here means the function-body import in the + diagnostics subsystem uses a wrong module path — a bug that would only + surface at runtime when the function is first called. + """ + try: + importlib.import_module(module_path) + except ModuleNotFoundError as exc: + pytest.fail( + f"{source_file}: deferred import '{label}' resolves to " + f"'{module_path}' which cannot be imported.\n" + f"Original error: {exc}" + ) + except Exception: + # Any other exception (ImportError subclass for missing optional + # dependency, circular import in unrelated code, etc.) is not our + # concern — we only care that the *module path* itself is valid. + pass + + +class TestDiagnosticsPackageImports: + """The diagnostics __init__.py must be importable and export all documented names.""" + + def test_package_is_importable(self): + """agent_notes.services.diagnostics must import without error.""" + mod = importlib.import_module("agent_notes.services.diagnostics") + assert mod is not None + + def test_checks_exports_are_present(self): + """All _checks functions must be present on the diagnostics package.""" + mod = importlib.import_module("agent_notes.services.diagnostics") + expected = [ + "check_stale_files", + "check_broken_symlinks", + "check_shadowed_files", + "check_missing_files", + "check_content_drift", + "check_build_freshness", + ] + for name in expected: + assert hasattr(mod, name), ( + f"agent_notes.services.diagnostics is missing exported name '{name}'" + ) + + def test_display_exports_are_present(self): + """All _display functions must be present on the diagnostics package.""" + mod = importlib.import_module("agent_notes.services.diagnostics") + expected = [ + "count_stale", + "print_summary", + "print_issues", + "_count_agents", + "_count_skills", + "_count_rules", + "_check_config", + "_check_role_models", + ] + for name in expected: + assert hasattr(mod, name), ( + f"agent_notes.services.diagnostics is missing exported name '{name}'" + ) + + def test_fix_export_is_present(self): + """do_fix must be present on the diagnostics package.""" + mod = importlib.import_module("agent_notes.services.diagnostics") + assert hasattr(mod, "do_fix"), ( + "agent_notes.services.diagnostics is missing exported name 'do_fix'" + ) + + def test_exported_names_are_callable(self): + """Every name in __all__ must be callable (a function).""" + mod = importlib.import_module("agent_notes.services.diagnostics") + for name in mod.__all__: + obj = getattr(mod, name, None) + assert callable(obj), ( + f"agent_notes.services.diagnostics.{name} is not callable (got {type(obj).__name__})" + ) + + +class TestDeferredImportCount: + """Guard that the AST extractor actually found deferred imports. + + If the extractor is broken (e.g. returns an empty list), all parametrised + tests would trivially pass — a false green. This test ensures we found a + meaningful set of imports to check. + """ + + def test_at_least_one_deferred_import_was_found(self): + assert len(_ALL_DEFERRED_IMPORTS) > 0, ( + "AST extractor found zero deferred imports — the extractor itself may be broken" + ) + + def test_deferred_imports_found_across_multiple_files(self): + source_files = {src for _mod, _label, src in _ALL_DEFERRED_IMPORTS} + assert len(source_files) >= 2, ( + f"Expected deferred imports from at least 2 source files; " + f"found only: {source_files}" + ) + + def test_agent_notes_root_modules_are_represented(self): + """At least one deferred import should be an agent_notes.* absolute path.""" + absolute_an = [ + mod for mod, _label, _src in _ALL_DEFERRED_IMPORTS + if mod.startswith("agent_notes.") + ] + assert len(absolute_an) > 0, ( + "No agent_notes.* absolute module paths found among deferred imports; " + "relative import resolution may be broken" + )