Skip to content

Commit 52f137c

Browse files
committed
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
1 parent 445eefe commit 52f137c

5 files changed

Lines changed: 98 additions & 82 deletions

File tree

scripts/bash/common.sh

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,9 +190,16 @@ except Exception:
190190
local candidate="$presets_dir/$preset_id/templates/${template_name}.md"
191191
[ -f "$candidate" ] && echo "$candidate" && return 0
192192
done <<< "$sorted_presets"
193+
else
194+
# python3 returned empty list — fall through to directory scan
195+
for preset in "$presets_dir"/*/; do
196+
[ -d "$preset" ] || continue
197+
local candidate="$preset/templates/${template_name}.md"
198+
[ -f "$candidate" ] && echo "$candidate" && return 0
199+
done
193200
fi
194201
else
195-
# Fallback: alphabetical directory order
202+
# Fallback: alphabetical directory order (no python3 available)
196203
for preset in "$presets_dir"/*/; do
197204
[ -d "$preset" ] || continue
198205
local candidate="$preset/templates/${template_name}.md"

scripts/powershell/common.ps1

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ function Resolve-Template {
189189
# Priority 3: Extension-provided templates
190190
$extDir = Join-Path $RepoRoot '.specify/extensions'
191191
if (Test-Path $extDir) {
192-
foreach ($ext in Get-ChildItem -Path $extDir -Directory -ErrorAction SilentlyContinue) {
192+
foreach ($ext in Get-ChildItem -Path $extDir -Directory -ErrorAction SilentlyContinue | Where-Object { $_.Name -notlike '.*' } | Sort-Object Name) {
193193
$candidate = Join-Path $ext.FullName "templates/$TemplateName.md"
194194
if (Test-Path $candidate) { return $candidate }
195195
}

src/specify_cli/__init__.py

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1574,6 +1574,19 @@ def init(
15741574
else:
15751575
tracker.skip("git", "--no-git flag")
15761576

1577+
# Persist the CLI options so later operations (e.g. preset add)
1578+
# can adapt their behaviour without re-scanning the filesystem.
1579+
# Must be saved BEFORE preset install so _get_skills_dir() works.
1580+
save_init_options(project_path, {
1581+
"ai": selected_ai,
1582+
"ai_skills": ai_skills,
1583+
"ai_commands_dir": ai_commands_dir,
1584+
"here": here,
1585+
"preset": preset,
1586+
"script": selected_script,
1587+
"speckit_version": get_speckit_version(),
1588+
})
1589+
15771590
# Install preset if specified
15781591
if preset:
15791592
try:
@@ -1590,23 +1603,16 @@ def init(
15901603
try:
15911604
zip_path = preset_catalog.download_pack(preset)
15921605
preset_manager.install_from_zip(zip_path, speckit_ver)
1606+
# Clean up downloaded ZIP to avoid cache accumulation
1607+
try:
1608+
zip_path.unlink(missing_ok=True)
1609+
except Exception:
1610+
pass
15931611
except PresetError:
15941612
console.print(f"[yellow]Warning:[/yellow] Preset '{preset}' not found in catalog. Skipping.")
15951613
except Exception as preset_err:
15961614
console.print(f"[yellow]Warning:[/yellow] Failed to install preset: {preset_err}")
15971615

1598-
# Persist the CLI options so later operations (e.g. preset add)
1599-
# can adapt their behaviour without re-scanning the filesystem.
1600-
save_init_options(project_path, {
1601-
"ai": selected_ai,
1602-
"ai_skills": ai_skills,
1603-
"ai_commands_dir": ai_commands_dir,
1604-
"here": here,
1605-
"preset": preset,
1606-
"script": selected_script,
1607-
"speckit_version": get_speckit_version(),
1608-
})
1609-
16101616
tracker.complete("final", "project ready")
16111617
except Exception as e:
16121618
tracker.error("final", str(e))
@@ -1957,6 +1963,14 @@ def preset_add(
19571963
console.print(f"[green]✓[/green] Preset '{manifest.name}' v{manifest.version} installed (priority {priority})")
19581964

19591965
elif from_url:
1966+
# Validate URL scheme before downloading
1967+
from urllib.parse import urlparse as _urlparse
1968+
_parsed = _urlparse(from_url)
1969+
_is_localhost = _parsed.hostname in ("localhost", "127.0.0.1", "::1")
1970+
if _parsed.scheme != "https" and not (_parsed.scheme == "http" and _is_localhost):
1971+
console.print(f"[red]Error:[/red] URL must use HTTPS (got {_parsed.scheme}://). HTTP is only allowed for localhost.")
1972+
raise typer.Exit(1)
1973+
19601974
console.print(f"Installing preset from [cyan]{from_url}[/cyan]...")
19611975
import urllib.request
19621976
import urllib.error

src/specify_cli/extensions.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -522,7 +522,7 @@ def remove(self, extension_id: str, keep_config: bool = False) -> bool:
522522
# Unregister commands from all AI agents
523523
if registered_commands:
524524
registrar = CommandRegistrar()
525-
registrar._registrar.unregister_commands(registered_commands, self.project_root)
525+
registrar.unregister_commands(registered_commands, self.project_root)
526526

527527
if keep_config:
528528
# Preserve config files, only remove non-config files
@@ -714,6 +714,14 @@ def register_commands_for_all_agents(
714714
context_note=context_note
715715
)
716716

717+
def unregister_commands(
718+
self,
719+
registered_commands: Dict[str, List[str]],
720+
project_root: Path
721+
) -> None:
722+
"""Remove previously registered command files from agent directories."""
723+
self._registrar.unregister_commands(registered_commands, project_root)
724+
717725
def register_commands_for_claude(
718726
self,
719727
manifest: ExtensionManifest,

src/specify_cli/presets.py

Lines changed: 54 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1137,14 +1137,16 @@ def fetch_catalog(self, force_refresh: bool = False) -> Dict[str, Any]:
11371137
Raises:
11381138
PresetError: If catalog cannot be fetched
11391139
"""
1140+
catalog_url = self.get_catalog_url()
1141+
11401142
if not force_refresh and self.is_cache_valid():
11411143
try:
1142-
return json.loads(self.cache_file.read_text())
1143-
except json.JSONDecodeError:
1144+
metadata = json.loads(self.cache_metadata_file.read_text())
1145+
if metadata.get("catalog_url") == catalog_url:
1146+
return json.loads(self.cache_file.read_text())
1147+
except (json.JSONDecodeError, OSError):
11441148
pass
11451149

1146-
catalog_url = self.get_catalog_url()
1147-
11481150
try:
11491151
import urllib.request
11501152
import urllib.error
@@ -1186,6 +1188,9 @@ def search(
11861188
) -> List[Dict[str, Any]]:
11871189
"""Search catalog for presets.
11881190
1191+
Searches across all active catalogs (merged by priority) so that
1192+
community and custom catalogs are included in results.
1193+
11891194
Args:
11901195
query: Search query (searches name, description, tags)
11911196
tag: Filter by specific tag
@@ -1195,12 +1200,11 @@ def search(
11951200
List of matching preset metadata
11961201
"""
11971202
try:
1198-
catalog_data = self.fetch_catalog()
1203+
packs = self._get_merged_packs()
11991204
except PresetError:
12001205
return []
12011206

12021207
results = []
1203-
packs = catalog_data.get("presets", {})
12041208

12051209
for pack_id, pack_data in packs.items():
12061210
if author and pack_data.get("author", "").lower() != author.lower():
@@ -1234,18 +1238,19 @@ def get_pack_info(
12341238
) -> Optional[Dict[str, Any]]:
12351239
"""Get detailed information about a specific preset.
12361240
1241+
Searches across all active catalogs (merged by priority).
1242+
12371243
Args:
12381244
pack_id: ID of the preset
12391245
12401246
Returns:
12411247
Pack metadata or None if not found
12421248
"""
12431249
try:
1244-
catalog_data = self.fetch_catalog()
1250+
packs = self._get_merged_packs()
12451251
except PresetError:
12461252
return None
12471253

1248-
packs = catalog_data.get("presets", {})
12491254
if pack_id in packs:
12501255
return {**packs[pack_id], "id": pack_id}
12511256
return None
@@ -1369,16 +1374,18 @@ def resolve(
13691374
else:
13701375
subdirs = [""]
13711376

1377+
# Determine file extension based on template type
1378+
ext = ".md"
1379+
if template_type == "script":
1380+
ext = ".sh" # scripts use .sh; callers can also check .ps1
1381+
13721382
# Priority 1: Project-local overrides
1373-
for subdir in subdirs:
1374-
if template_type == "script":
1375-
override = self.overrides_dir / "scripts" / f"{template_name}.sh"
1376-
elif subdir:
1377-
override = self.overrides_dir / f"{template_name}.md"
1378-
else:
1379-
override = self.overrides_dir / f"{template_name}.md"
1380-
if override.exists():
1381-
return override
1383+
if template_type == "script":
1384+
override = self.overrides_dir / "scripts" / f"{template_name}{ext}"
1385+
else:
1386+
override = self.overrides_dir / f"{template_name}{ext}"
1387+
if override.exists():
1388+
return override
13821389

13831390
# Priority 2: Installed presets (sorted by priority — lower number wins)
13841391
if self.presets_dir.exists():
@@ -1387,11 +1394,9 @@ def resolve(
13871394
pack_dir = self.presets_dir / pack_id
13881395
for subdir in subdirs:
13891396
if subdir:
1390-
candidate = (
1391-
pack_dir / subdir / f"{template_name}.md"
1392-
)
1397+
candidate = pack_dir / subdir / f"{template_name}{ext}"
13931398
else:
1394-
candidate = pack_dir / f"{template_name}.md"
1399+
candidate = pack_dir / f"{template_name}{ext}"
13951400
if candidate.exists():
13961401
return candidate
13971402

@@ -1402,13 +1407,9 @@ def resolve(
14021407
continue
14031408
for subdir in subdirs:
14041409
if subdir:
1405-
candidate = (
1406-
ext_dir / "templates" / f"{template_name}.md"
1407-
)
1410+
candidate = ext_dir / subdir / f"{template_name}{ext}"
14081411
else:
1409-
candidate = (
1410-
ext_dir / "templates" / f"{template_name}.md"
1411-
)
1412+
candidate = ext_dir / "templates" / f"{template_name}{ext}"
14121413
if candidate.exists():
14131414
return candidate
14141415

@@ -1421,6 +1422,10 @@ def resolve(
14211422
core = self.templates_dir / "commands" / f"{template_name}.md"
14221423
if core.exists():
14231424
return core
1425+
elif template_type == "script":
1426+
core = self.templates_dir / "scripts" / f"{template_name}{ext}"
1427+
if core.exists():
1428+
return core
14241429

14251430
return None
14261431

@@ -1438,52 +1443,34 @@ def resolve_with_source(
14381443
Returns:
14391444
Dictionary with 'path' and 'source' keys, or None if not found
14401445
"""
1441-
# Priority 1: Project-local overrides
1442-
override = self.overrides_dir / f"{template_name}.md"
1443-
if override.exists():
1444-
return {"path": str(override), "source": "project override"}
1446+
# Delegate to resolve() for the actual lookup, then determine source
1447+
resolved = self.resolve(template_name, template_type)
1448+
if resolved is None:
1449+
return None
14451450

1446-
# Priority 2: Installed presets (sorted by priority — lower number wins)
1447-
if self.presets_dir.exists():
1451+
resolved_str = str(resolved)
1452+
1453+
# Determine source attribution
1454+
if str(self.overrides_dir) in resolved_str:
1455+
return {"path": resolved_str, "source": "project override"}
1456+
1457+
if str(self.presets_dir) in resolved_str and self.presets_dir.exists():
14481458
registry = PresetRegistry(self.presets_dir)
14491459
for pack_id, _metadata in registry.list_by_priority():
1450-
pack_dir = self.presets_dir / pack_id
1451-
# Check templates/ subdirectory first, then root
1452-
for subdir in ["templates", "commands", "scripts", ""]:
1453-
if subdir:
1454-
candidate = (
1455-
pack_dir / subdir / f"{template_name}.md"
1456-
)
1457-
else:
1458-
candidate = pack_dir / f"{template_name}.md"
1459-
if candidate.exists():
1460-
meta = registry.get(pack_id)
1461-
version = meta.get("version", "?") if meta else "?"
1462-
return {
1463-
"path": str(candidate),
1464-
"source": f"{pack_id} v{version}",
1465-
}
1460+
if f"/{pack_id}/" in resolved_str:
1461+
meta = registry.get(pack_id)
1462+
version = meta.get("version", "?") if meta else "?"
1463+
return {
1464+
"path": resolved_str,
1465+
"source": f"{pack_id} v{version}",
1466+
}
14661467

1467-
# Priority 3: Extension-provided templates
1468-
if self.extensions_dir.exists():
1468+
if str(self.extensions_dir) in resolved_str and self.extensions_dir.exists():
14691469
for ext_dir in sorted(self.extensions_dir.iterdir()):
1470-
if not ext_dir.is_dir() or ext_dir.name.startswith("."):
1471-
continue
1472-
candidate = ext_dir / "templates" / f"{template_name}.md"
1473-
if candidate.exists():
1470+
if ext_dir.is_dir() and f"/{ext_dir.name}/" in resolved_str:
14741471
return {
1475-
"path": str(candidate),
1472+
"path": resolved_str,
14761473
"source": f"extension:{ext_dir.name}",
14771474
}
14781475

1479-
# Priority 4: Core templates
1480-
core = self.templates_dir / f"{template_name}.md"
1481-
if core.exists():
1482-
return {"path": str(core), "source": "core"}
1483-
1484-
# Also check commands subdirectory for core
1485-
core_cmd = self.templates_dir / "commands" / f"{template_name}.md"
1486-
if core_cmd.exists():
1487-
return {"path": str(core_cmd), "source": "core"}
1488-
1489-
return None
1476+
return {"path": resolved_str, "source": "core"}

0 commit comments

Comments
 (0)