mirror of
https://github.com/github/spec-kit.git
synced 2026-03-24 14:23:09 +00:00
fix: address PR review — legacy teardown, generic agent, ~/.specify paths, 3-segment commands_dir, full file tracking
- Legacy --ai teardown: detect empty tracked files and fall back to AGENT_CONFIG-based directory removal during agent switch - --agent generic: falls through to legacy flow (no embedded pack) - User/catalog dirs: use ~/.specify/ instead of platformdirs for consistency with extensions/presets - DefaultBootstrap: join all path segments after first for COMMANDS_SUBDIR (fixes 3+-segment commands_dir like .tabnine/agent/commands) - agent_add --from: validate manifest.id matches provided agent_id - finalize_setup: track all files from setup(), not just agent-root files - setup() docstring: reference --agent not --ai - AGENTS.md: document generic agent fallback behavior
This commit is contained in:
@@ -440,7 +440,7 @@ specify init my-project --agent claude # Pack-based flow (with file tra
|
|||||||
specify init --here --agent gemini --ai-skills # With skills
|
specify init --here --agent gemini --ai-skills # With skills
|
||||||
```
|
```
|
||||||
|
|
||||||
`--agent` and `--ai` are mutually exclusive. When `--agent` is used, `init-options.json` gains `"agent_pack": true`.
|
`--agent` and `--ai` are mutually exclusive. When `--agent` is used, `init-options.json` gains `"agent_pack": true`. The `generic` agent (which requires `--ai-commands-dir`) falls through to the legacy flow since it has no embedded pack.
|
||||||
|
|
||||||
### `specify agent` subcommands
|
### `specify agent` subcommands
|
||||||
|
|
||||||
|
|||||||
@@ -1715,7 +1715,7 @@ def _handle_agent_skills_migration(console: Console, agent_key: str) -> None:
|
|||||||
def init(
|
def init(
|
||||||
project_name: str = typer.Argument(None, help="Name for your new project directory (optional if using --here, or use '.' for current directory)"),
|
project_name: str = typer.Argument(None, help="Name for your new project directory (optional if using --here, or use '.' for current directory)"),
|
||||||
ai_assistant: str = typer.Option(None, "--ai", help=AI_ASSISTANT_HELP),
|
ai_assistant: str = typer.Option(None, "--ai", help=AI_ASSISTANT_HELP),
|
||||||
agent: str = typer.Option(None, "--agent", help="AI agent to use (enables file tracking for clean teardown when switching agents). Accepts the same agent IDs as --ai."),
|
agent: str = typer.Option(None, "--agent", help="AI agent to use (enables file tracking for clean teardown when switching agents). Accepts the same agent IDs as --ai. Use --ai generic for custom agent directories."),
|
||||||
ai_commands_dir: str = typer.Option(None, "--ai-commands-dir", help="Directory for agent command files (required with --ai generic, e.g. .myagent/commands/)"),
|
ai_commands_dir: str = typer.Option(None, "--ai-commands-dir", help="Directory for agent command files (required with --ai generic, e.g. .myagent/commands/)"),
|
||||||
script_type: str = typer.Option(None, "--script", help="Script type to use: sh or ps"),
|
script_type: str = typer.Option(None, "--script", help="Script type to use: sh or ps"),
|
||||||
ignore_agent_tools: bool = typer.Option(False, "--ignore-agent-tools", help="Skip checks for AI agent tools like Claude Code"),
|
ignore_agent_tools: bool = typer.Option(False, "--ignore-agent-tools", help="Skip checks for AI agent tools like Claude Code"),
|
||||||
@@ -1784,7 +1784,10 @@ def init(
|
|||||||
console.print("[red]Error:[/red] --agent and --ai cannot both be specified. Use one or the other.")
|
console.print("[red]Error:[/red] --agent and --ai cannot both be specified. Use one or the other.")
|
||||||
raise typer.Exit(1)
|
raise typer.Exit(1)
|
||||||
ai_assistant = agent
|
ai_assistant = agent
|
||||||
use_agent_pack = True
|
# "generic" uses --ai-commands-dir and has no embedded pack,
|
||||||
|
# so it falls through to the legacy flow.
|
||||||
|
if agent != "generic":
|
||||||
|
use_agent_pack = True
|
||||||
|
|
||||||
# Detect when option values are likely misinterpreted flags (parameter ordering issue)
|
# Detect when option values are likely misinterpreted flags (parameter ordering issue)
|
||||||
if ai_assistant and ai_assistant.startswith("--"):
|
if ai_assistant and ai_assistant.startswith("--"):
|
||||||
@@ -2678,15 +2681,28 @@ def agent_switch(
|
|||||||
old_tracked_agent, old_tracked_ext = get_tracked_files(project_path, current_agent)
|
old_tracked_agent, old_tracked_ext = get_tracked_files(project_path, current_agent)
|
||||||
all_files = {**old_tracked_agent, **old_tracked_ext}
|
all_files = {**old_tracked_agent, **old_tracked_ext}
|
||||||
|
|
||||||
console.print(f" [dim]Tearing down {current_agent}...[/dim]")
|
if all_files:
|
||||||
current_bootstrap.teardown(
|
console.print(f" [dim]Tearing down {current_agent}...[/dim]")
|
||||||
project_path,
|
current_bootstrap.teardown(
|
||||||
force=True, # already confirmed above
|
project_path,
|
||||||
files=all_files if all_files else None,
|
force=True, # already confirmed above
|
||||||
)
|
files=all_files,
|
||||||
console.print(f" [green]✓[/green] {current_agent} removed")
|
)
|
||||||
|
console.print(f" [green]✓[/green] {current_agent} removed")
|
||||||
|
else:
|
||||||
|
# No install manifest (legacy --ai project) — fall back
|
||||||
|
# to removing the agent directory via AGENT_CONFIG.
|
||||||
|
agent_config = AGENT_CONFIG.get(current_agent, {})
|
||||||
|
agent_folder = agent_config.get("folder")
|
||||||
|
if agent_folder:
|
||||||
|
agent_dir = project_path / agent_folder.rstrip("/")
|
||||||
|
if agent_dir.is_dir():
|
||||||
|
shutil.rmtree(agent_dir)
|
||||||
|
console.print(f" [green]✓[/green] {current_agent} directory removed (legacy)")
|
||||||
|
else:
|
||||||
|
console.print(f" [yellow]Warning:[/yellow] No tracked files or AGENT_CONFIG entry for '{current_agent}' — skipping teardown")
|
||||||
except AgentPackError:
|
except AgentPackError:
|
||||||
# If pack-based teardown fails, try legacy cleanup via AGENT_CONFIG
|
# If pack-based resolution/load fails, try legacy cleanup via AGENT_CONFIG
|
||||||
agent_config = AGENT_CONFIG.get(current_agent, {})
|
agent_config = AGENT_CONFIG.get(current_agent, {})
|
||||||
agent_folder = agent_config.get("folder")
|
agent_folder = agent_config.get("folder")
|
||||||
if agent_folder:
|
if agent_folder:
|
||||||
@@ -2925,6 +2941,13 @@ def agent_add(
|
|||||||
console.print(f"[red]Validation failed:[/red] {exc}")
|
console.print(f"[red]Validation failed:[/red] {exc}")
|
||||||
raise typer.Exit(1)
|
raise typer.Exit(1)
|
||||||
|
|
||||||
|
if manifest.id != agent_id:
|
||||||
|
console.print(
|
||||||
|
f"[red]Error:[/red] Manifest ID '{manifest.id}' does not match "
|
||||||
|
f"the specified agent ID '{agent_id}'."
|
||||||
|
)
|
||||||
|
raise typer.Exit(1)
|
||||||
|
|
||||||
dest = _catalog_agents_dir() / manifest.id
|
dest = _catalog_agents_dir() / manifest.id
|
||||||
dest.mkdir(parents=True, exist_ok=True)
|
dest.mkdir(parents=True, exist_ok=True)
|
||||||
shutil.copytree(source, dest, dirs_exist_ok=True)
|
shutil.copytree(source, dest, dirs_exist_ok=True)
|
||||||
|
|||||||
@@ -25,7 +25,6 @@ from pathlib import Path
|
|||||||
from typing import Any, Dict, List, Optional
|
from typing import Any, Dict, List, Optional
|
||||||
|
|
||||||
import yaml
|
import yaml
|
||||||
from platformdirs import user_data_path
|
|
||||||
|
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
@@ -212,8 +211,10 @@ class AgentBootstrap:
|
|||||||
def setup(self, project_path: Path, script_type: str, options: Dict[str, Any]) -> List[Path]:
|
def setup(self, project_path: Path, script_type: str, options: Dict[str, Any]) -> List[Path]:
|
||||||
"""Install agent files into *project_path*.
|
"""Install agent files into *project_path*.
|
||||||
|
|
||||||
This is invoked by ``specify init --ai <agent>`` and
|
This is invoked by ``specify init --agent <agent>`` and
|
||||||
``specify agent switch <agent>``.
|
``specify agent switch <agent>``. The legacy ``--ai`` flag
|
||||||
|
uses the old non-pack bootstrap flow and does not call this
|
||||||
|
method.
|
||||||
|
|
||||||
Implementations **must** return every file they create so that the
|
Implementations **must** return every file they create so that the
|
||||||
CLI can record both agent-installed files and extension-installed
|
CLI can record both agent-installed files and extension-installed
|
||||||
@@ -340,37 +341,20 @@ class AgentBootstrap:
|
|||||||
|
|
||||||
This must be called **after** the full init pipeline has finished
|
This must be called **after** the full init pipeline has finished
|
||||||
writing files (commands, context files, extensions) into the
|
writing files (commands, context files, extensions) into the
|
||||||
project. It combines the files reported by :meth:`setup` with
|
project. It records every file reported by :meth:`setup` plus
|
||||||
any extra files (e.g. from extension registration), scans the
|
any extra files (e.g. from extension registration) and scans
|
||||||
agent's directory tree for anything additional, and writes the
|
the agent's directory tree for anything additional.
|
||||||
install manifest.
|
|
||||||
|
|
||||||
``setup()`` may return *all* files created by the shared
|
All files returned by ``setup()`` are tracked — including shared
|
||||||
scaffolding (including shared project files in ``.specify/``).
|
project infrastructure — so that teardown/switch can precisely
|
||||||
Only files under the agent's own directory tree are recorded as
|
remove everything the agent installed.
|
||||||
``agent_files`` — shared project infrastructure is not tracked
|
|
||||||
per-agent and will not be removed during teardown.
|
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
agent_files: Files reported by :meth:`setup`.
|
agent_files: Files reported by :meth:`setup`.
|
||||||
extension_files: Files created by extension registration.
|
extension_files: Files created by extension registration.
|
||||||
"""
|
"""
|
||||||
all_extension = list(extension_files or [])
|
all_extension = list(extension_files or [])
|
||||||
|
all_agent: List[Path] = list(agent_files or [])
|
||||||
# Filter agent_files: only keep files under the agent's directory
|
|
||||||
# tree. setup() returns *all* scaffolded files (including shared
|
|
||||||
# project infrastructure in .specify/) but only agent-owned files
|
|
||||||
# should be tracked per-agent — shared files are not removed
|
|
||||||
# during teardown/switch.
|
|
||||||
agent_root = self.agent_dir(project_path)
|
|
||||||
agent_root_resolved = agent_root.resolve()
|
|
||||||
all_agent: List[Path] = []
|
|
||||||
for p in (agent_files or []):
|
|
||||||
try:
|
|
||||||
p.resolve().relative_to(agent_root_resolved)
|
|
||||||
all_agent.append(p)
|
|
||||||
except ValueError:
|
|
||||||
pass # Path is outside the agent root — skip it
|
|
||||||
|
|
||||||
# Scan the agent's directory tree for files created by later
|
# Scan the agent's directory tree for files created by later
|
||||||
# init pipeline steps (skills, presets, extensions) that
|
# init pipeline steps (skills, presets, extensions) that
|
||||||
@@ -413,7 +397,7 @@ class DefaultBootstrap(AgentBootstrap):
|
|||||||
super().__init__(manifest)
|
super().__init__(manifest)
|
||||||
parts = manifest.commands_dir.split("/") if manifest.commands_dir else []
|
parts = manifest.commands_dir.split("/") if manifest.commands_dir else []
|
||||||
self.AGENT_DIR = parts[0] if parts else ""
|
self.AGENT_DIR = parts[0] if parts else ""
|
||||||
self.COMMANDS_SUBDIR = parts[1] if len(parts) > 1 else ""
|
self.COMMANDS_SUBDIR = "/".join(parts[1:]) if len(parts) > 1 else ""
|
||||||
|
|
||||||
def setup(self, project_path: Path, script_type: str, options: Dict[str, Any]) -> List[Path]:
|
def setup(self, project_path: Path, script_type: str, options: Dict[str, Any]) -> List[Path]:
|
||||||
"""Install agent files into the project using the standard scaffold."""
|
"""Install agent files into the project using the standard scaffold."""
|
||||||
@@ -673,7 +657,7 @@ def _embedded_agents_dir() -> Path:
|
|||||||
|
|
||||||
def _user_agents_dir() -> Path:
|
def _user_agents_dir() -> Path:
|
||||||
"""Return the user-level agent overrides directory."""
|
"""Return the user-level agent overrides directory."""
|
||||||
return user_data_path("specify", "github") / "agents"
|
return Path.home() / ".specify" / "agents"
|
||||||
|
|
||||||
|
|
||||||
def _project_agents_dir(project_path: Path) -> Path:
|
def _project_agents_dir(project_path: Path) -> Path:
|
||||||
@@ -683,7 +667,7 @@ def _project_agents_dir(project_path: Path) -> Path:
|
|||||||
|
|
||||||
def _catalog_agents_dir() -> Path:
|
def _catalog_agents_dir() -> Path:
|
||||||
"""Return the catalog-installed agent cache directory."""
|
"""Return the catalog-installed agent cache directory."""
|
||||||
return user_data_path("specify", "github") / "agent-cache"
|
return Path.home() / ".specify" / "agent-cache"
|
||||||
|
|
||||||
|
|
||||||
@dataclass
|
@dataclass
|
||||||
|
|||||||
Reference in New Issue
Block a user