mirror of
https://github.com/github/spec-kit.git
synced 2026-03-22 13:23:08 +00:00
fix: address Copilot PR review comments
- Move save_init_options() before preset install so skills propagation works during 'specify init --preset --ai-skills' - Clean up downloaded ZIP after successful preset install during init - Validate --from URL scheme (require HTTPS, HTTP only for localhost) - Expose unregister_commands() on extensions.py CommandRegistrar wrapper instead of reaching into private _registrar field - Use _get_merged_packs() for search() and get_pack_info() so all active catalogs are searched, not just the highest-priority one - Fix fetch_catalog() cache to verify cached URL matches current URL - Fix PresetResolver: script resolution uses .sh extension, consistent file extensions throughout resolve(), and resolve_with_source() delegates to resolve() to honor template_type parameter - Fix bash common.sh: fall through to directory scan when python3 returns empty preset list - Fix PowerShell Resolve-Template: filter out dot-folders and sort extensions deterministically
This commit is contained in:
@@ -190,9 +190,16 @@ except Exception:
|
|||||||
local candidate="$presets_dir/$preset_id/templates/${template_name}.md"
|
local candidate="$presets_dir/$preset_id/templates/${template_name}.md"
|
||||||
[ -f "$candidate" ] && echo "$candidate" && return 0
|
[ -f "$candidate" ] && echo "$candidate" && return 0
|
||||||
done <<< "$sorted_presets"
|
done <<< "$sorted_presets"
|
||||||
|
else
|
||||||
|
# python3 returned empty list — fall through to directory scan
|
||||||
|
for preset in "$presets_dir"/*/; do
|
||||||
|
[ -d "$preset" ] || continue
|
||||||
|
local candidate="$preset/templates/${template_name}.md"
|
||||||
|
[ -f "$candidate" ] && echo "$candidate" && return 0
|
||||||
|
done
|
||||||
fi
|
fi
|
||||||
else
|
else
|
||||||
# Fallback: alphabetical directory order
|
# Fallback: alphabetical directory order (no python3 available)
|
||||||
for preset in "$presets_dir"/*/; do
|
for preset in "$presets_dir"/*/; do
|
||||||
[ -d "$preset" ] || continue
|
[ -d "$preset" ] || continue
|
||||||
local candidate="$preset/templates/${template_name}.md"
|
local candidate="$preset/templates/${template_name}.md"
|
||||||
|
|||||||
@@ -189,7 +189,7 @@ function Resolve-Template {
|
|||||||
# Priority 3: Extension-provided templates
|
# Priority 3: Extension-provided templates
|
||||||
$extDir = Join-Path $RepoRoot '.specify/extensions'
|
$extDir = Join-Path $RepoRoot '.specify/extensions'
|
||||||
if (Test-Path $extDir) {
|
if (Test-Path $extDir) {
|
||||||
foreach ($ext in Get-ChildItem -Path $extDir -Directory -ErrorAction SilentlyContinue) {
|
foreach ($ext in Get-ChildItem -Path $extDir -Directory -ErrorAction SilentlyContinue | Where-Object { $_.Name -notlike '.*' } | Sort-Object Name) {
|
||||||
$candidate = Join-Path $ext.FullName "templates/$TemplateName.md"
|
$candidate = Join-Path $ext.FullName "templates/$TemplateName.md"
|
||||||
if (Test-Path $candidate) { return $candidate }
|
if (Test-Path $candidate) { return $candidate }
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1574,6 +1574,19 @@ def init(
|
|||||||
else:
|
else:
|
||||||
tracker.skip("git", "--no-git flag")
|
tracker.skip("git", "--no-git flag")
|
||||||
|
|
||||||
|
# Persist the CLI options so later operations (e.g. preset add)
|
||||||
|
# can adapt their behaviour without re-scanning the filesystem.
|
||||||
|
# Must be saved BEFORE preset install so _get_skills_dir() works.
|
||||||
|
save_init_options(project_path, {
|
||||||
|
"ai": selected_ai,
|
||||||
|
"ai_skills": ai_skills,
|
||||||
|
"ai_commands_dir": ai_commands_dir,
|
||||||
|
"here": here,
|
||||||
|
"preset": preset,
|
||||||
|
"script": selected_script,
|
||||||
|
"speckit_version": get_speckit_version(),
|
||||||
|
})
|
||||||
|
|
||||||
# Install preset if specified
|
# Install preset if specified
|
||||||
if preset:
|
if preset:
|
||||||
try:
|
try:
|
||||||
@@ -1590,23 +1603,16 @@ def init(
|
|||||||
try:
|
try:
|
||||||
zip_path = preset_catalog.download_pack(preset)
|
zip_path = preset_catalog.download_pack(preset)
|
||||||
preset_manager.install_from_zip(zip_path, speckit_ver)
|
preset_manager.install_from_zip(zip_path, speckit_ver)
|
||||||
|
# Clean up downloaded ZIP to avoid cache accumulation
|
||||||
|
try:
|
||||||
|
zip_path.unlink(missing_ok=True)
|
||||||
|
except Exception:
|
||||||
|
pass
|
||||||
except PresetError:
|
except PresetError:
|
||||||
console.print(f"[yellow]Warning:[/yellow] Preset '{preset}' not found in catalog. Skipping.")
|
console.print(f"[yellow]Warning:[/yellow] Preset '{preset}' not found in catalog. Skipping.")
|
||||||
except Exception as preset_err:
|
except Exception as preset_err:
|
||||||
console.print(f"[yellow]Warning:[/yellow] Failed to install preset: {preset_err}")
|
console.print(f"[yellow]Warning:[/yellow] Failed to install preset: {preset_err}")
|
||||||
|
|
||||||
# Persist the CLI options so later operations (e.g. preset add)
|
|
||||||
# can adapt their behaviour without re-scanning the filesystem.
|
|
||||||
save_init_options(project_path, {
|
|
||||||
"ai": selected_ai,
|
|
||||||
"ai_skills": ai_skills,
|
|
||||||
"ai_commands_dir": ai_commands_dir,
|
|
||||||
"here": here,
|
|
||||||
"preset": preset,
|
|
||||||
"script": selected_script,
|
|
||||||
"speckit_version": get_speckit_version(),
|
|
||||||
})
|
|
||||||
|
|
||||||
tracker.complete("final", "project ready")
|
tracker.complete("final", "project ready")
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
tracker.error("final", str(e))
|
tracker.error("final", str(e))
|
||||||
@@ -1957,6 +1963,14 @@ def preset_add(
|
|||||||
console.print(f"[green]✓[/green] Preset '{manifest.name}' v{manifest.version} installed (priority {priority})")
|
console.print(f"[green]✓[/green] Preset '{manifest.name}' v{manifest.version} installed (priority {priority})")
|
||||||
|
|
||||||
elif from_url:
|
elif from_url:
|
||||||
|
# Validate URL scheme before downloading
|
||||||
|
from urllib.parse import urlparse as _urlparse
|
||||||
|
_parsed = _urlparse(from_url)
|
||||||
|
_is_localhost = _parsed.hostname in ("localhost", "127.0.0.1", "::1")
|
||||||
|
if _parsed.scheme != "https" and not (_parsed.scheme == "http" and _is_localhost):
|
||||||
|
console.print(f"[red]Error:[/red] URL must use HTTPS (got {_parsed.scheme}://). HTTP is only allowed for localhost.")
|
||||||
|
raise typer.Exit(1)
|
||||||
|
|
||||||
console.print(f"Installing preset from [cyan]{from_url}[/cyan]...")
|
console.print(f"Installing preset from [cyan]{from_url}[/cyan]...")
|
||||||
import urllib.request
|
import urllib.request
|
||||||
import urllib.error
|
import urllib.error
|
||||||
|
|||||||
@@ -522,7 +522,7 @@ class ExtensionManager:
|
|||||||
# Unregister commands from all AI agents
|
# Unregister commands from all AI agents
|
||||||
if registered_commands:
|
if registered_commands:
|
||||||
registrar = CommandRegistrar()
|
registrar = CommandRegistrar()
|
||||||
registrar._registrar.unregister_commands(registered_commands, self.project_root)
|
registrar.unregister_commands(registered_commands, self.project_root)
|
||||||
|
|
||||||
if keep_config:
|
if keep_config:
|
||||||
# Preserve config files, only remove non-config files
|
# Preserve config files, only remove non-config files
|
||||||
@@ -714,6 +714,14 @@ class CommandRegistrar:
|
|||||||
context_note=context_note
|
context_note=context_note
|
||||||
)
|
)
|
||||||
|
|
||||||
|
def unregister_commands(
|
||||||
|
self,
|
||||||
|
registered_commands: Dict[str, List[str]],
|
||||||
|
project_root: Path
|
||||||
|
) -> None:
|
||||||
|
"""Remove previously registered command files from agent directories."""
|
||||||
|
self._registrar.unregister_commands(registered_commands, project_root)
|
||||||
|
|
||||||
def register_commands_for_claude(
|
def register_commands_for_claude(
|
||||||
self,
|
self,
|
||||||
manifest: ExtensionManifest,
|
manifest: ExtensionManifest,
|
||||||
|
|||||||
@@ -1137,14 +1137,16 @@ class PresetCatalog:
|
|||||||
Raises:
|
Raises:
|
||||||
PresetError: If catalog cannot be fetched
|
PresetError: If catalog cannot be fetched
|
||||||
"""
|
"""
|
||||||
|
catalog_url = self.get_catalog_url()
|
||||||
|
|
||||||
if not force_refresh and self.is_cache_valid():
|
if not force_refresh and self.is_cache_valid():
|
||||||
try:
|
try:
|
||||||
return json.loads(self.cache_file.read_text())
|
metadata = json.loads(self.cache_metadata_file.read_text())
|
||||||
except json.JSONDecodeError:
|
if metadata.get("catalog_url") == catalog_url:
|
||||||
|
return json.loads(self.cache_file.read_text())
|
||||||
|
except (json.JSONDecodeError, OSError):
|
||||||
pass
|
pass
|
||||||
|
|
||||||
catalog_url = self.get_catalog_url()
|
|
||||||
|
|
||||||
try:
|
try:
|
||||||
import urllib.request
|
import urllib.request
|
||||||
import urllib.error
|
import urllib.error
|
||||||
@@ -1186,6 +1188,9 @@ class PresetCatalog:
|
|||||||
) -> List[Dict[str, Any]]:
|
) -> List[Dict[str, Any]]:
|
||||||
"""Search catalog for presets.
|
"""Search catalog for presets.
|
||||||
|
|
||||||
|
Searches across all active catalogs (merged by priority) so that
|
||||||
|
community and custom catalogs are included in results.
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
query: Search query (searches name, description, tags)
|
query: Search query (searches name, description, tags)
|
||||||
tag: Filter by specific tag
|
tag: Filter by specific tag
|
||||||
@@ -1195,12 +1200,11 @@ class PresetCatalog:
|
|||||||
List of matching preset metadata
|
List of matching preset metadata
|
||||||
"""
|
"""
|
||||||
try:
|
try:
|
||||||
catalog_data = self.fetch_catalog()
|
packs = self._get_merged_packs()
|
||||||
except PresetError:
|
except PresetError:
|
||||||
return []
|
return []
|
||||||
|
|
||||||
results = []
|
results = []
|
||||||
packs = catalog_data.get("presets", {})
|
|
||||||
|
|
||||||
for pack_id, pack_data in packs.items():
|
for pack_id, pack_data in packs.items():
|
||||||
if author and pack_data.get("author", "").lower() != author.lower():
|
if author and pack_data.get("author", "").lower() != author.lower():
|
||||||
@@ -1234,6 +1238,8 @@ class PresetCatalog:
|
|||||||
) -> Optional[Dict[str, Any]]:
|
) -> Optional[Dict[str, Any]]:
|
||||||
"""Get detailed information about a specific preset.
|
"""Get detailed information about a specific preset.
|
||||||
|
|
||||||
|
Searches across all active catalogs (merged by priority).
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
pack_id: ID of the preset
|
pack_id: ID of the preset
|
||||||
|
|
||||||
@@ -1241,11 +1247,10 @@ class PresetCatalog:
|
|||||||
Pack metadata or None if not found
|
Pack metadata or None if not found
|
||||||
"""
|
"""
|
||||||
try:
|
try:
|
||||||
catalog_data = self.fetch_catalog()
|
packs = self._get_merged_packs()
|
||||||
except PresetError:
|
except PresetError:
|
||||||
return None
|
return None
|
||||||
|
|
||||||
packs = catalog_data.get("presets", {})
|
|
||||||
if pack_id in packs:
|
if pack_id in packs:
|
||||||
return {**packs[pack_id], "id": pack_id}
|
return {**packs[pack_id], "id": pack_id}
|
||||||
return None
|
return None
|
||||||
@@ -1369,16 +1374,18 @@ class PresetResolver:
|
|||||||
else:
|
else:
|
||||||
subdirs = [""]
|
subdirs = [""]
|
||||||
|
|
||||||
|
# Determine file extension based on template type
|
||||||
|
ext = ".md"
|
||||||
|
if template_type == "script":
|
||||||
|
ext = ".sh" # scripts use .sh; callers can also check .ps1
|
||||||
|
|
||||||
# Priority 1: Project-local overrides
|
# Priority 1: Project-local overrides
|
||||||
for subdir in subdirs:
|
if template_type == "script":
|
||||||
if template_type == "script":
|
override = self.overrides_dir / "scripts" / f"{template_name}{ext}"
|
||||||
override = self.overrides_dir / "scripts" / f"{template_name}.sh"
|
else:
|
||||||
elif subdir:
|
override = self.overrides_dir / f"{template_name}{ext}"
|
||||||
override = self.overrides_dir / f"{template_name}.md"
|
if override.exists():
|
||||||
else:
|
return override
|
||||||
override = self.overrides_dir / f"{template_name}.md"
|
|
||||||
if override.exists():
|
|
||||||
return override
|
|
||||||
|
|
||||||
# Priority 2: Installed presets (sorted by priority — lower number wins)
|
# Priority 2: Installed presets (sorted by priority — lower number wins)
|
||||||
if self.presets_dir.exists():
|
if self.presets_dir.exists():
|
||||||
@@ -1387,11 +1394,9 @@ class PresetResolver:
|
|||||||
pack_dir = self.presets_dir / pack_id
|
pack_dir = self.presets_dir / pack_id
|
||||||
for subdir in subdirs:
|
for subdir in subdirs:
|
||||||
if subdir:
|
if subdir:
|
||||||
candidate = (
|
candidate = pack_dir / subdir / f"{template_name}{ext}"
|
||||||
pack_dir / subdir / f"{template_name}.md"
|
|
||||||
)
|
|
||||||
else:
|
else:
|
||||||
candidate = pack_dir / f"{template_name}.md"
|
candidate = pack_dir / f"{template_name}{ext}"
|
||||||
if candidate.exists():
|
if candidate.exists():
|
||||||
return candidate
|
return candidate
|
||||||
|
|
||||||
@@ -1402,13 +1407,9 @@ class PresetResolver:
|
|||||||
continue
|
continue
|
||||||
for subdir in subdirs:
|
for subdir in subdirs:
|
||||||
if subdir:
|
if subdir:
|
||||||
candidate = (
|
candidate = ext_dir / subdir / f"{template_name}{ext}"
|
||||||
ext_dir / "templates" / f"{template_name}.md"
|
|
||||||
)
|
|
||||||
else:
|
else:
|
||||||
candidate = (
|
candidate = ext_dir / "templates" / f"{template_name}{ext}"
|
||||||
ext_dir / "templates" / f"{template_name}.md"
|
|
||||||
)
|
|
||||||
if candidate.exists():
|
if candidate.exists():
|
||||||
return candidate
|
return candidate
|
||||||
|
|
||||||
@@ -1421,6 +1422,10 @@ class PresetResolver:
|
|||||||
core = self.templates_dir / "commands" / f"{template_name}.md"
|
core = self.templates_dir / "commands" / f"{template_name}.md"
|
||||||
if core.exists():
|
if core.exists():
|
||||||
return core
|
return core
|
||||||
|
elif template_type == "script":
|
||||||
|
core = self.templates_dir / "scripts" / f"{template_name}{ext}"
|
||||||
|
if core.exists():
|
||||||
|
return core
|
||||||
|
|
||||||
return None
|
return None
|
||||||
|
|
||||||
@@ -1438,52 +1443,34 @@ class PresetResolver:
|
|||||||
Returns:
|
Returns:
|
||||||
Dictionary with 'path' and 'source' keys, or None if not found
|
Dictionary with 'path' and 'source' keys, or None if not found
|
||||||
"""
|
"""
|
||||||
# Priority 1: Project-local overrides
|
# Delegate to resolve() for the actual lookup, then determine source
|
||||||
override = self.overrides_dir / f"{template_name}.md"
|
resolved = self.resolve(template_name, template_type)
|
||||||
if override.exists():
|
if resolved is None:
|
||||||
return {"path": str(override), "source": "project override"}
|
return None
|
||||||
|
|
||||||
# Priority 2: Installed presets (sorted by priority — lower number wins)
|
resolved_str = str(resolved)
|
||||||
if self.presets_dir.exists():
|
|
||||||
|
# Determine source attribution
|
||||||
|
if str(self.overrides_dir) in resolved_str:
|
||||||
|
return {"path": resolved_str, "source": "project override"}
|
||||||
|
|
||||||
|
if str(self.presets_dir) in resolved_str and self.presets_dir.exists():
|
||||||
registry = PresetRegistry(self.presets_dir)
|
registry = PresetRegistry(self.presets_dir)
|
||||||
for pack_id, _metadata in registry.list_by_priority():
|
for pack_id, _metadata in registry.list_by_priority():
|
||||||
pack_dir = self.presets_dir / pack_id
|
if f"/{pack_id}/" in resolved_str:
|
||||||
# Check templates/ subdirectory first, then root
|
meta = registry.get(pack_id)
|
||||||
for subdir in ["templates", "commands", "scripts", ""]:
|
version = meta.get("version", "?") if meta else "?"
|
||||||
if subdir:
|
|
||||||
candidate = (
|
|
||||||
pack_dir / subdir / f"{template_name}.md"
|
|
||||||
)
|
|
||||||
else:
|
|
||||||
candidate = pack_dir / f"{template_name}.md"
|
|
||||||
if candidate.exists():
|
|
||||||
meta = registry.get(pack_id)
|
|
||||||
version = meta.get("version", "?") if meta else "?"
|
|
||||||
return {
|
|
||||||
"path": str(candidate),
|
|
||||||
"source": f"{pack_id} v{version}",
|
|
||||||
}
|
|
||||||
|
|
||||||
# Priority 3: Extension-provided templates
|
|
||||||
if self.extensions_dir.exists():
|
|
||||||
for ext_dir in sorted(self.extensions_dir.iterdir()):
|
|
||||||
if not ext_dir.is_dir() or ext_dir.name.startswith("."):
|
|
||||||
continue
|
|
||||||
candidate = ext_dir / "templates" / f"{template_name}.md"
|
|
||||||
if candidate.exists():
|
|
||||||
return {
|
return {
|
||||||
"path": str(candidate),
|
"path": resolved_str,
|
||||||
|
"source": f"{pack_id} v{version}",
|
||||||
|
}
|
||||||
|
|
||||||
|
if str(self.extensions_dir) in resolved_str and self.extensions_dir.exists():
|
||||||
|
for ext_dir in sorted(self.extensions_dir.iterdir()):
|
||||||
|
if ext_dir.is_dir() and f"/{ext_dir.name}/" in resolved_str:
|
||||||
|
return {
|
||||||
|
"path": resolved_str,
|
||||||
"source": f"extension:{ext_dir.name}",
|
"source": f"extension:{ext_dir.name}",
|
||||||
}
|
}
|
||||||
|
|
||||||
# Priority 4: Core templates
|
return {"path": resolved_str, "source": "core"}
|
||||||
core = self.templates_dir / f"{template_name}.md"
|
|
||||||
if core.exists():
|
|
||||||
return {"path": str(core), "source": "core"}
|
|
||||||
|
|
||||||
# Also check commands subdirectory for core
|
|
||||||
core_cmd = self.templates_dir / "commands" / f"{template_name}.md"
|
|
||||||
if core_cmd.exists():
|
|
||||||
return {"path": str(core_cmd), "source": "core"}
|
|
||||||
|
|
||||||
return None
|
|
||||||
|
|||||||
Reference in New Issue
Block a user