diff --git a/src/lola/cli/install.py b/src/lola/cli/install.py index 79c8463..1a75314 100644 --- a/src/lola/cli/install.py +++ b/src/lola/cli/install.py @@ -46,6 +46,7 @@ get_target, install_to_assistant, ) +from lola.targets.install import _install_instructions from lola.utils import ensure_lola_dirs, get_local_modules_path from lola.cli.utils import handle_lola_error @@ -527,11 +528,12 @@ def _update_instructions(ctx: UpdateContext, verbose: bool) -> bool: Returns True if instructions were successfully installed. """ - from lola.models import INSTRUCTIONS_FILE - path_context = ctx.inst.project_path or "" scope = ctx.inst.scope + if not ctx.inst.install_instructions: + return False + if not ctx.has_instructions: # Always attempt removal - handles stale installation records instructions_dest = ctx.target.get_instructions_path(path_context, scope) @@ -540,35 +542,19 @@ def _update_instructions(ctx: UpdateContext, verbose: bool) -> bool: console.print(" [yellow]- instructions[/yellow] [dim](removed)[/dim]") return False - instructions_dest = ctx.target.get_instructions_path(path_context, scope) - - # Respect --append-context from the original installation - if ctx.inst.append_context: - from lola.targets.install import _install_instructions - - success = _install_instructions( - ctx.target, - ctx.global_module, - ctx.source_module, - ctx.inst.project_path, - ctx.inst.append_context, - scope, - ) - if success and verbose: - console.print(" [green]instructions (appended)[/green]") - return success - - content_path = _get_content_path(ctx.source_module) - instructions_source = content_path / INSTRUCTIONS_FILE - if not instructions_source.exists(): - return False - - success = ctx.target.generate_instructions( - instructions_source, instructions_dest, ctx.inst.module_name + success = _install_instructions( + ctx.target, + ctx.global_module, + ctx.source_module, + ctx.inst.project_path, + ctx.inst.append_context, + scope, + install_instructions=True, ) if success and verbose: - console.print(" [green]instructions[/green]") + label = "instructions (appended)" if ctx.inst.append_context else "instructions" + console.print(f" [green]{label}[/green]") return success @@ -736,6 +722,13 @@ def _format_update_summary(result: UpdateResult) -> str: "Pass the path to the main context file relative to the module root " "(e.g., module/AGENTS.md).", ) +@click.option( + "--instructions/--no-instructions", + default=None, + help="Install module instructions into assistant instruction files. " + "By default, existing instruction files are prompted for interactively " + "and skipped in non-interactive mode.", +) @click.option( "--workspace", type=str, @@ -760,6 +753,7 @@ def install_cmd( pre_install: Optional[str], post_install: Optional[str], append_context: Optional[str], + instructions: Optional[bool], workspace: Optional[str], scope: str, project_path: str, @@ -784,6 +778,11 @@ def install_cmd( """ project_path = _resolve_install_path(assistant, project_path, workspace) + if append_context and instructions is False: + raise click.UsageError("--append-context cannot be used with --no-instructions") + if append_context and instructions is None: + instructions = True + ensure_lola_dirs() # Resolve module_name interactively when omitted @@ -938,6 +937,7 @@ def install_cmd( effective_pre_install, effective_post_install, append_context, + instructions, ) # Update installation records with version from marketplace metadata @@ -1346,7 +1346,8 @@ def update_cmd(module_name: Optional[str], assistant: Optional[str], verbose: bo inst.commands = list(ctx.current_commands) inst.agents = list(ctx.current_agents) inst.mcps = list(ctx.current_mcps) - inst.has_instructions = result.instructions_ok + if inst.install_instructions: + inst.has_instructions = result.instructions_ok registry.add(inst) # Print summary line for this installation diff --git a/src/lola/models.py b/src/lola/models.py index b08d571..ede1696 100644 --- a/src/lola/models.py +++ b/src/lola/models.py @@ -551,6 +551,7 @@ class Installation: mcps: list[str] = field(default_factory=list) has_instructions: bool = False append_context: Optional[str] = None + install_instructions: bool = True def to_dict(self) -> dict: """Convert to dictionary for YAML serialization.""" @@ -563,6 +564,7 @@ def to_dict(self) -> dict: "agents": self.agents, "mcps": self.mcps, "has_instructions": self.has_instructions, + "install_instructions": self.install_instructions, } if self.project_path: result["project_path"] = self.project_path @@ -587,6 +589,7 @@ def from_dict(cls, data: dict) -> "Installation": mcps=data.get("mcps", []), has_instructions=data.get("has_instructions", False), append_context=data.get("append_context"), + install_instructions=data.get("install_instructions", False), ) diff --git a/src/lola/prompts.py b/src/lola/prompts.py index 55a9c8f..41f3c41 100644 --- a/src/lola/prompts.py +++ b/src/lola/prompts.py @@ -170,3 +170,12 @@ def prompt_agent_conflict(agent_name: str, module_name: str) -> tuple[str, str]: ).execute() return "rename", str(new_name) return str(action) if action is not None else "skip", "" + + +def confirm_replace_instructions(instructions_path: str) -> bool: + """Prompt before writing to an existing assistant instructions file.""" + result = inquirer.confirm( + message=f"{instructions_path} already exists. Install Lola instructions there?", + default=False, + ).execute() + return bool(result) diff --git a/src/lola/targets/base.py b/src/lola/targets/base.py index bc7710a..121f6a6 100644 --- a/src/lola/targets/base.py +++ b/src/lola/targets/base.py @@ -557,10 +557,11 @@ def remove_skill(self, dest_path: Path, skill_name: str) -> bool: class ManagedInstructionsTarget: - """Mixin for targets that use managed sections for module instructions. + """Mixin for targets that write module instructions to assistant files. - This provides shared logic for inserting/removing module instructions - into markdown files like CLAUDE.md, GEMINI.md, AGENTS.md. + New installs replace files like CLAUDE.md, GEMINI.md, and AGENTS.md in full. + Removal still understands legacy managed sections so older installations can + be cleaned up. """ INSTRUCTIONS_START_MARKER: str = "" @@ -580,72 +581,13 @@ def generate_instructions( dest_path: Path, module_name: str, ) -> bool: - """Generate/update module instructions in a managed section.""" + """Replace the assistant instructions file with module instructions.""" instructions_content = _resolve_source_content(source) if not instructions_content: return False - # Read existing file content - if dest_path.exists(): - content = dest_path.read_text() - else: - dest_path.parent.mkdir(parents=True, exist_ok=True) - content = "" - - module_start, module_end = self._get_module_markers(module_name) - - # Build the module block - module_block = f"{module_start}\n{instructions_content}\n{module_end}" - - # Check if managed section exists - if ( - self.INSTRUCTIONS_START_MARKER in content - and self.INSTRUCTIONS_END_MARKER in content - ): - start_idx = content.index(self.INSTRUCTIONS_START_MARKER) - end_idx = content.index(self.INSTRUCTIONS_END_MARKER) + len( - self.INSTRUCTIONS_END_MARKER - ) - existing_section = content[start_idx:end_idx] - section_content = existing_section[ - len(self.INSTRUCTIONS_START_MARKER) : -len(self.INSTRUCTIONS_END_MARKER) - ] - - # Remove existing module section if present - if module_start in section_content: - mod_start_idx = section_content.index(module_start) - mod_end_idx = section_content.index(module_end) + len(module_end) - section_content = ( - section_content[:mod_start_idx] + section_content[mod_end_idx:] - ) - - # Collect all module blocks and sort them alphabetically - module_blocks = self._extract_module_blocks(section_content) - module_blocks[module_name] = module_block - - # Build new section with sorted modules - sorted_blocks = [ - module_blocks[name] for name in sorted(module_blocks.keys()) - ] - new_section_content = "\n\n".join(sorted_blocks) - if new_section_content: - new_section_content = "\n" + new_section_content + "\n" - - new_section = ( - self.INSTRUCTIONS_START_MARKER - + new_section_content - + self.INSTRUCTIONS_END_MARKER - ) - content = content[:start_idx] + new_section + content[end_idx:] - else: - # Create new managed section at the end - new_section = ( - f"\n\n{self.INSTRUCTIONS_START_MARKER}\n{module_block}\n" - f"{self.INSTRUCTIONS_END_MARKER}\n" - ) - content = content.rstrip() + new_section - - dest_path.write_text(content) + dest_path.parent.mkdir(parents=True, exist_ok=True) + dest_path.write_text(instructions_content) return True def _extract_module_blocks(self, section_content: str) -> dict[str, str]: @@ -661,14 +603,14 @@ def _extract_module_blocks(self, section_content: str) -> dict[str, str]: def remove_instructions(self, dest_path: Path, module_name: str) -> bool: """Remove a module's instructions from the managed section.""" if not dest_path.exists(): - return True + return False content = dest_path.read_text() if ( self.INSTRUCTIONS_START_MARKER not in content or self.INSTRUCTIONS_END_MARKER not in content ): - return True + return False module_start, module_end = self._get_module_markers(module_name) diff --git a/src/lola/targets/install.py b/src/lola/targets/install.py index 39f5b62..4d4954a 100644 --- a/src/lola/targets/install.py +++ b/src/lola/targets/install.py @@ -23,7 +23,12 @@ import lola.config as config from lola.exceptions import InstallationError from lola.models import Installation, InstallationRegistry, Module -from lola.prompts import is_interactive, prompt_agent_conflict, prompt_command_conflict +from lola.prompts import ( + confirm_replace_instructions, + is_interactive, + prompt_agent_conflict, + prompt_command_conflict, +) from .base import ( AssistantTarget, @@ -354,6 +359,7 @@ def _install_instructions( project_path: str | None, append_context: str | None = None, scope: str = "project", + install_instructions: bool | None = None, ) -> bool: """Install module instructions for a target. Returns True if installed.""" from lola.models import INSTRUCTIONS_FILE @@ -361,12 +367,31 @@ def _install_instructions( if not module.has_instructions: return False + if install_instructions is False: + return False + if scope == "project" and not project_path: return False # Type checker: at this point project_path is guaranteed to be a string instructions_dest = target.get_instructions_path(cast(str, project_path), scope) + if ( + install_instructions is None + and instructions_dest.exists() + and not instructions_dest.is_dir() + and not append_context + ): + if not is_interactive(): + click.echo( + f"Instructions already exist at {instructions_dest}; " + "pass --instructions to install instructions.", + err=True, + ) + return False + if not confirm_replace_instructions(str(instructions_dest)): + return False + # --append-context: insert a reference instead of verbatim copy if append_context: context_file = local_module_path / append_context @@ -384,10 +409,6 @@ def _install_instructions( reference = f"Read the module context from `{relative_path}`" return target.generate_instructions(reference, instructions_dest, module.name) - # Default: verbatim copy of AGENTS.md - if not module.has_instructions: - return False - content_dirname = _get_content_dirname(module) content_path = _get_content_path(local_module_path, content_dirname) instructions_source = content_path / INSTRUCTIONS_FILE @@ -517,12 +538,23 @@ def install_to_assistant( pre_install_script: Optional[str] = None, post_install_script: Optional[str] = None, append_context: Optional[str] = None, + install_instructions: Optional[bool] = None, ) -> int: """Install module to a specific assistant.""" # Late import to avoid circular imports - get_target is defined in __init__.py from lola.targets import get_target target = get_target(assistant) + existing_installation = next( + ( + inst + for inst in registry.find(module.name) + if inst.assistant == assistant + and inst.scope == scope + and inst.project_path == project_path + ), + None, + ) local_module_path = copy_module_to_local(module, local_modules) @@ -555,7 +587,13 @@ def install_to_assistant( target, module, local_module_path, project_path, scope ) instructions_installed = _install_instructions( - target, module, local_module_path, project_path, append_context, scope + target, + module, + local_module_path, + project_path, + append_context, + scope, + install_instructions, ) _print_summary( @@ -580,6 +618,19 @@ def install_to_assistant( or installed_mcps or instructions_installed ): + if install_instructions is False: + keep_installing_instructions = False + has_instructions = False + elif instructions_installed or install_instructions is True: + keep_installing_instructions = True + has_instructions = instructions_installed + elif existing_installation: + keep_installing_instructions = existing_installation.install_instructions + has_instructions = existing_installation.has_instructions + else: + keep_installing_instructions = not module.has_instructions + has_instructions = False + registry.add( Installation( module_name=module.name, @@ -590,8 +641,9 @@ def install_to_assistant( commands=installed_commands, agents=installed_agents, mcps=installed_mcps, - has_instructions=instructions_installed, + has_instructions=has_instructions, append_context=append_context, + install_instructions=keep_installing_instructions, ) ) @@ -712,6 +764,12 @@ def _uninstall_instructions( path_context = inst.project_path or "" scope = inst.scope instructions_dest = target.get_instructions_path(path_context, scope) + + if inst.install_instructions and instructions_dest.exists(): + if not instructions_dest.is_dir(): + instructions_dest.unlink() + return True + return target.remove_instructions(instructions_dest, inst.module_name) diff --git a/tests/test_instructions.py b/tests/test_instructions.py index 3ba06c9..9624a7d 100644 --- a/tests/test_instructions.py +++ b/tests/test_instructions.py @@ -173,6 +173,27 @@ def test_from_dict_defaults_append_context_to_none(self): inst = Installation.from_dict(data) assert inst.append_context is None + def test_to_dict_includes_install_instructions(self): + """Installation.to_dict() includes install_instructions.""" + inst = Installation( + module_name="test", + assistant="claude-code", + scope="project", + install_instructions=False, + ) + data = inst.to_dict() + assert data["install_instructions"] is False + + def test_from_dict_defaults_install_instructions_to_false(self): + """Legacy records require explicit opt-in before replacing instructions.""" + data = { + "module": "test", + "assistant": "claude-code", + "scope": "project", + } + inst = Installation.from_dict(data) + assert inst.install_instructions is False + # ============================================================================= # _resolve_source_content Tests @@ -216,7 +237,7 @@ def test_get_instructions_path(self, tmp_path): assert path == tmp_path / "CLAUDE.md" def test_generate_instructions_creates_file(self, tmp_path): - """generate_instructions creates CLAUDE.md with managed section.""" + """generate_instructions creates CLAUDE.md from module instructions.""" target = ClaudeCodeTarget() source = tmp_path / "source" / "AGENTS.md" source.parent.mkdir() @@ -228,15 +249,10 @@ def test_generate_instructions_creates_file(self, tmp_path): assert result is True assert dest.exists() - content = dest.read_text() - assert "" in content - assert "" in content - assert "" in content - assert "" in content - assert "# Test Module" in content + assert dest.read_text() == "# Test Module\n\nThese are instructions." - def test_generate_instructions_preserves_existing_content(self, tmp_path): - """generate_instructions preserves existing content outside managed section.""" + def test_generate_instructions_replaces_existing_content(self, tmp_path): + """generate_instructions replaces existing assistant instructions.""" target = ClaudeCodeTarget() source = tmp_path / "source" / "AGENTS.md" source.parent.mkdir() @@ -249,13 +265,10 @@ def test_generate_instructions_preserves_existing_content(self, tmp_path): result = target.generate_instructions(source, dest, "new-module") assert result is True - content = dest.read_text() - assert "# My Project" in content - assert "Existing content here." in content - assert "# New Module" in content + assert dest.read_text() == "# New Module\n\nNew instructions." - def test_generate_instructions_multiple_modules(self, tmp_path): - """generate_instructions handles multiple modules sorted alphabetically.""" + def test_generate_instructions_replaces_previous_module(self, tmp_path): + """generate_instructions replaces previous module instructions.""" target = ClaudeCodeTarget() dest = tmp_path / "project" / "CLAUDE.md" @@ -272,13 +285,11 @@ def test_generate_instructions_multiple_modules(self, tmp_path): target.generate_instructions(source2, dest, "alpha-module") content = dest.read_text() - # Alpha should come before Zeta (sorted) - alpha_pos = content.find("alpha-module") - zeta_pos = content.find("zeta-module") - assert alpha_pos < zeta_pos + assert content == "# Alpha Module" + assert "# Zeta Module" not in content def test_remove_instructions(self, tmp_path): - """remove_instructions removes module and cleans up empty section.""" + """remove_instructions is a no-op for full-file instructions.""" target = ClaudeCodeTarget() dest = tmp_path / "CLAUDE.md" @@ -291,28 +302,27 @@ def test_remove_instructions(self, tmp_path): # Then remove result = target.remove_instructions(dest, "test-module") - assert result is True + assert result is False content = dest.read_text() - assert "test-module" not in content - # Managed section markers should be removed when empty + assert "# Test Module" in content assert "" not in content assert "" not in content - def test_remove_instructions_keeps_other_modules(self, tmp_path): - """remove_instructions keeps section when other modules remain.""" + def test_remove_instructions_cleans_legacy_managed_section(self, tmp_path): + """remove_instructions still cleans legacy managed instruction blocks.""" target = ClaudeCodeTarget() dest = tmp_path / "CLAUDE.md" - - # Add two modules - source1 = tmp_path / "source1" / "AGENTS.md" - source1.parent.mkdir() - source1.write_text("# Module One") - target.generate_instructions(source1, dest, "module-one") - - source2 = tmp_path / "source2" / "AGENTS.md" - source2.parent.mkdir() - source2.write_text("# Module Two") - target.generate_instructions(source2, dest, "module-two") + dest.write_text( + "# My Project\n\n" + "\n" + "\n" + "# Module One\n" + "\n\n" + "\n" + "# Module Two\n" + "\n" + "\n" + ) # Remove one module result = target.remove_instructions(dest, "module-one") @@ -321,7 +331,6 @@ def test_remove_instructions_keeps_other_modules(self, tmp_path): content = dest.read_text() assert "module-one" not in content assert "module-two" in content - # Managed section markers should still exist assert "" in content def test_generate_instructions_empty_source_returns_false(self, tmp_path): @@ -420,8 +429,8 @@ def test_get_instructions_path(self, tmp_path): path = target.get_instructions_path(str(tmp_path)) assert path == tmp_path / "GEMINI.md" - def test_generate_instructions_creates_managed_section(self, tmp_path): - """generate_instructions creates managed section in GEMINI.md.""" + def test_generate_instructions_replaces_file(self, tmp_path): + """generate_instructions replaces GEMINI.md.""" target = GeminiTarget() source = tmp_path / "source" / "AGENTS.md" source.parent.mkdir() @@ -432,10 +441,7 @@ def test_generate_instructions_creates_managed_section(self, tmp_path): result = target.generate_instructions(source, dest, "test-module") assert result is True - content = dest.read_text() - assert "" in content - assert "" in content - assert "# Test Module" in content + assert dest.read_text() == "# Test Module\n\nInstructions." # ============================================================================= @@ -452,8 +458,8 @@ def test_get_instructions_path(self, tmp_path): path = target.get_instructions_path(str(tmp_path)) assert path == tmp_path / "AGENTS.md" - def test_generate_instructions_creates_managed_section(self, tmp_path): - """generate_instructions creates managed section in AGENTS.md.""" + def test_generate_instructions_replaces_file(self, tmp_path): + """generate_instructions replaces AGENTS.md.""" target = OpenCodeTarget() source = tmp_path / "source" / "AGENTS.md" source.parent.mkdir() @@ -464,9 +470,7 @@ def test_generate_instructions_creates_managed_section(self, tmp_path): result = target.generate_instructions(source, dest, "test-module") assert result is True - content = dest.read_text() - assert "" in content - assert "" in content + assert dest.read_text() == "# Test Module\n\nInstructions." # ============================================================================= @@ -509,6 +513,182 @@ def test_install_with_instructions(self, tmp_path): assert result.exit_code == 0 assert "instructions" in result.output + def test_non_interactive_existing_instructions_skips_with_stderr( + self, tmp_path, capsys + ): + """Existing instruction files are skipped in non-interactive installs.""" + from lola.targets.install import _install_instructions + + module_dir = tmp_path / "test-module" + module_dir.mkdir() + (module_dir / "AGENTS.md").write_text("# Test Module\n\nInstructions.") + module = Module.from_path(module_dir) + assert module is not None + + project_dir = tmp_path / "project" + project_dir.mkdir() + claude_md = project_dir / "CLAUDE.md" + claude_md.write_text("# My Instructions\n") + + with patch("lola.targets.install.is_interactive", return_value=False): + result = _install_instructions( + ClaudeCodeTarget(), module, module_dir, str(project_dir) + ) + + captured = capsys.readouterr() + assert result is False + assert "Instructions already exist" in captured.err + assert "pass --instructions" in captured.err + assert claude_md.read_text() == "# My Instructions\n" + + def test_no_instructions_skips_existing_instructions_quietly( + self, tmp_path, capsys + ): + """--no-instructions skips without warning or file changes.""" + from lola.targets.install import _install_instructions + + module_dir = tmp_path / "test-module" + module_dir.mkdir() + (module_dir / "AGENTS.md").write_text("# Test Module\n\nInstructions.") + module = Module.from_path(module_dir) + assert module is not None + + project_dir = tmp_path / "project" + project_dir.mkdir() + claude_md = project_dir / "CLAUDE.md" + claude_md.write_text("# My Instructions\n") + + result = _install_instructions( + ClaudeCodeTarget(), + module, + module_dir, + str(project_dir), + install_instructions=False, + ) + + captured = capsys.readouterr() + assert result is False + assert captured.err == "" + assert claude_md.read_text() == "# My Instructions\n" + + def test_instructions_flag_replaces_existing_instructions(self, tmp_path): + """--instructions forces installation into an existing instruction file.""" + from lola.targets.install import _install_instructions + + module_dir = tmp_path / "test-module" + module_dir.mkdir() + (module_dir / "AGENTS.md").write_text("# Test Module\n\nInstructions.") + module = Module.from_path(module_dir) + assert module is not None + + project_dir = tmp_path / "project" + project_dir.mkdir() + claude_md = project_dir / "CLAUDE.md" + claude_md.write_text("# My Instructions\n") + + result = _install_instructions( + ClaudeCodeTarget(), + module, + module_dir, + str(project_dir), + install_instructions=True, + ) + + content = claude_md.read_text() + assert result is True + assert "# My Instructions" not in content + assert "" not in content + assert "# Test Module" in content + + def test_interactive_existing_instructions_prompts(self, tmp_path): + """Interactive installs ask before writing to an existing instruction file.""" + from lola.targets.install import _install_instructions + + module_dir = tmp_path / "test-module" + module_dir.mkdir() + (module_dir / "AGENTS.md").write_text("# Test Module\n\nInstructions.") + module = Module.from_path(module_dir) + assert module is not None + + project_dir = tmp_path / "project" + project_dir.mkdir() + claude_md = project_dir / "CLAUDE.md" + claude_md.write_text("# My Instructions\n") + + with ( + patch("lola.targets.install.is_interactive", return_value=True), + patch( + "lola.targets.install.confirm_replace_instructions", + return_value=True, + ) as confirm, + ): + result = _install_instructions( + ClaudeCodeTarget(), module, module_dir, str(project_dir) + ) + + assert result is True + confirm.assert_called_once_with(str(claude_md)) + assert "# Test Module" in claude_md.read_text() + + def test_install_records_skipped_instructions_preference(self, tmp_path): + """Skipped instruction installs are remembered for future updates.""" + from lola.targets.install import install_to_assistant + + module_dir = tmp_path / "test-module" + module_dir.mkdir() + skills_dir = module_dir / "skills" / "skill1" + skills_dir.mkdir(parents=True) + (skills_dir / "SKILL.md").write_text("---\ndescription: Test\n---\nContent") + (module_dir / "AGENTS.md").write_text("# Test Module\n\nInstructions.") + module = Module.from_path(module_dir) + assert module is not None + + project_dir = tmp_path / "project" + project_dir.mkdir() + (project_dir / "CLAUDE.md").write_text("# My Instructions\n") + registry = InstallationRegistry(tmp_path / "installed.yml") + + with patch("lola.targets.install.is_interactive", return_value=False): + count = install_to_assistant( + module, + "claude-code", + "project", + str(project_dir), + project_dir / ".lola" / "modules", + registry, + ) + + inst = registry.find("test-module")[0] + assert count == 1 + assert inst.has_instructions is False + assert inst.install_instructions is False + + def test_existing_cursor_rules_directory_does_not_skip_instructions( + self, tmp_path, capsys + ): + """Directory-based instruction targets do not trigger shared-file prompts.""" + from lola.targets.install import _install_instructions + + module_dir = tmp_path / "test-module" + module_dir.mkdir() + (module_dir / "AGENTS.md").write_text("# Test Module\n\nInstructions.") + module = Module.from_path(module_dir) + assert module is not None + + project_dir = tmp_path / "project" + rules_dir = project_dir / ".cursor" / "rules" + rules_dir.mkdir(parents=True) + + with patch("lola.targets.install.is_interactive", return_value=False): + result = _install_instructions( + CursorTarget(), module, module_dir, str(project_dir) + ) + + captured = capsys.readouterr() + assert result is True + assert captured.err == "" + assert (rules_dir / "test-module-instructions.mdc").exists() + class TestUninstallWithInstructions: """Tests for uninstall command with instructions.""" @@ -518,7 +698,7 @@ def test_uninstall_removes_instructions(self, tmp_path): from lola.cli.install import uninstall_cmd installed_file = tmp_path / ".lola" / "installed.yml" - installed_file.parent.mkdir(parents=True) + installed_file.parent.mkdir(parents=True, exist_ok=True) # Create registry with installation that has instructions registry = InstallationRegistry(installed_file) @@ -529,6 +709,7 @@ def test_uninstall_removes_instructions(self, tmp_path): project_path=str(tmp_path / "project"), skills=["test-module-skill1"], has_instructions=True, + install_instructions=False, ) registry.add(inst) @@ -570,6 +751,30 @@ def test_uninstall_removes_instructions(self, tmp_path): assert result.exit_code == 0 mock_target.remove_instructions.assert_called_once() + def test_uninstall_deletes_full_file_instructions(self, tmp_path): + """Uninstall deletes assistant files managed by full-file installs.""" + from lola.targets import ClaudeCodeTarget + from lola.targets.install import _uninstall_instructions + + project_dir = tmp_path / "project" + project_dir.mkdir() + claude_md = project_dir / "CLAUDE.md" + claude_md.write_text("# Managed Module Instructions\n") + + inst = Installation( + module_name="test-module", + assistant="claude-code", + scope="project", + project_path=str(project_dir), + has_instructions=True, + install_instructions=True, + ) + + result = _uninstall_instructions(ClaudeCodeTarget(), inst) + + assert result is True + assert not claude_md.exists() + class TestUpdateWithInstructions: """Tests for update command with instructions.""" @@ -649,6 +854,75 @@ def test_update_regenerates_instructions(self, tmp_path): assert "instructions" in result.output mock_target.generate_instructions.assert_called() + def test_update_legacy_record_does_not_replace_instructions(self, tmp_path): + """Legacy records must not replace instruction files without opt-in.""" + from lola.cli.install import update_cmd + + modules_dir = tmp_path / ".lola" / "modules" + modules_dir.mkdir(parents=True) + installed_file = tmp_path / ".lola" / "installed.yml" + + module_dir = modules_dir / "test-module" + module_dir.mkdir() + skills_dir = module_dir / "skills" / "skill1" + skills_dir.mkdir(parents=True) + (skills_dir / "SKILL.md").write_text("---\ndescription: Test\n---\nContent") + (module_dir / "AGENTS.md").write_text("# Updated Instructions") + + project_dir = tmp_path / "project" + project_dir.mkdir() + installed_file.parent.mkdir(parents=True, exist_ok=True) + installed_file.write_text( + "\n".join( + [ + "version: '1.0'", + "installations:", + "- module: test-module", + " assistant: claude-code", + " scope: project", + " skills:", + " - skill1", + " commands: []", + " agents: []", + " mcps: []", + " has_instructions: true", + f" project_path: {project_dir}", + "", + ] + ) + ) + registry = InstallationRegistry(installed_file) + + local_modules = project_dir / ".lola" / "modules" + local_modules.mkdir(parents=True) + shutil.copytree(module_dir, local_modules / "test-module") + + mock_target = MagicMock() + mock_target.uses_managed_section = False + mock_target.supports_agents = True + mock_target.get_skill_path.return_value = project_dir / ".claude" / "skills" + mock_target.get_command_path.return_value = project_dir / ".claude" / "commands" + mock_target.get_agent_path.return_value = None + mock_target.get_instructions_path.return_value = project_dir / "CLAUDE.md" + mock_target.generate_skill.return_value = True + + runner = CliRunner() + with ( + patch("lola.cli.install.MODULES_DIR", modules_dir), + patch("lola.cli.install.ensure_lola_dirs"), + patch("lola.cli.install.get_registry", return_value=registry), + patch( + "lola.cli.install.get_local_modules_path", return_value=local_modules + ), + patch("lola.cli.install.get_target", return_value=mock_target), + ): + result = runner.invoke(update_cmd, ["test-module"]) + + assert result.exit_code == 0 + mock_target.generate_instructions.assert_not_called() + assert registry.find("test-module")[0].install_instructions is False + assert registry.find("test-module")[0].has_instructions is True + def test_update_installs_new_instructions(self, tmp_path): """Update installs instructions if module now has AGENTS.md.""" from lola.cli.install import update_cmd @@ -849,8 +1123,7 @@ def test_update_existing_module_instructions(self, tmp_path): content = dest.read_text() assert "# Version 2" in content assert "# Version 1" not in content - # Should only have one module section - assert content.count("lola:module:test-module:start") == 1 + assert "lola:module:test-module:start" not in content # ============================================================================= @@ -872,19 +1145,14 @@ def test_remove_last_module_cleans_up_markers(self, tmp_path): target = ClaudeCodeTarget() dest = tmp_path / "CLAUDE.md" - # Add some existing content before instructions - dest.write_text("# My Project\n\nSome content here.\n\n") - - # Add instructions - source = tmp_path / "source" / "AGENTS.md" - source.parent.mkdir() - source.write_text("# Module Instructions") - target.generate_instructions(source, dest, "test-module") - - # Verify instructions were added - content = dest.read_text() - assert "" in content - assert "test-module" in content + dest.write_text( + "# My Project\n\nSome content here.\n\n" + "\n" + "\n" + "# Module Instructions\n" + "\n" + "\n" + ) # Remove the module target.remove_instructions(dest, "test-module") @@ -1039,7 +1307,7 @@ def test_append_context_creates_reference(self, tmp_path): content = claude_md.read_text() assert "Read the module context from" in content assert ".lola/modules/test-module/module/AGENTS.md" in content - assert "" in content + assert "" not in content def test_append_context_missing_file_returns_false(self, tmp_path): """--append-context returns False when the context file doesn't exist.""" @@ -1063,8 +1331,8 @@ def test_append_context_missing_file_returns_false(self, tmp_path): assert result is False - def test_append_context_preserves_existing_claude_md(self, tmp_path): - """--append-context preserves existing content in CLAUDE.md.""" + def test_append_context_replaces_existing_claude_md(self, tmp_path): + """--append-context replaces existing content in CLAUDE.md.""" from lola.targets.install import _install_instructions target = ClaudeCodeTarget() @@ -1097,8 +1365,8 @@ def test_append_context_preserves_existing_claude_md(self, tmp_path): assert result is True content = claude_md.read_text() - assert "# My Project" in content - assert "Existing content." in content + assert "# My Project" not in content + assert "Existing content." not in content assert "Read the module context from" in content def test_default_behavior_unchanged_without_flag(self, tmp_path):