Skip to content

Commit a9a7594

Browse files
minbang930codex
andauthored
fix: recover active skills registration for extensions (#2803)
Extension command registration now resolves the active skills directory before writing command artifacts. This lets initialized skills-backed agents recover a missing active skills directory while preserving the existing preset registration behavior. Add regression coverage for missing active skills directories, shared skills directories, and symlinked parent guards. Fixes #2769. Co-authored-by: OpenAI Codex <codex@openai.com>
1 parent 8e5643d commit a9a7594

16 files changed

Lines changed: 824 additions & 104 deletions

src/specify_cli/__init__.py

Lines changed: 14 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,12 @@
8686
DEFAULT_INIT_INTEGRATION as DEFAULT_INIT_INTEGRATION,
8787
SCRIPT_TYPE_CHOICES as SCRIPT_TYPE_CHOICES,
8888
)
89+
from ._init_options import (
90+
INIT_OPTIONS_FILE as INIT_OPTIONS_FILE,
91+
is_ai_skills_enabled as _is_ai_skills_enabled,
92+
load_init_options as load_init_options,
93+
save_init_options as save_init_options,
94+
)
8995

9096
app = typer.Typer(
9197
name="specify",
@@ -259,65 +265,6 @@ def ensure_executable_scripts(project_path: Path, tracker: StepTracker | None =
259265
for f in failures:
260266
console.print(f" - {f}")
261267

262-
INIT_OPTIONS_FILE = ".specify/init-options.json"
263-
264-
265-
def save_init_options(project_path: Path, options: dict[str, Any]) -> None:
266-
"""Persist the CLI options used during ``specify init``.
267-
268-
Writes a small JSON file to ``.specify/init-options.json`` so that
269-
later operations (e.g. preset install) can adapt their behaviour
270-
without scanning the filesystem.
271-
"""
272-
dest = project_path / INIT_OPTIONS_FILE
273-
dest.parent.mkdir(parents=True, exist_ok=True)
274-
# Write JSON as real UTF-8 instead of ``\uXXXX`` escape sequences
275-
# (``ensure_ascii=False``) and pin the file encoding to match.
276-
#
277-
# The default ``json.dumps`` output is ASCII-only — any non-ASCII
278-
# character is encoded as a ``\uXXXX`` escape — so without the
279-
# ``ensure_ascii=False`` flip below the encoding pin alone would be
280-
# a no-op for any payload we plausibly write today. We pair the two
281-
# so the on-disk bytes match a human's expectation of "this file is
282-
# UTF-8" (greppable, readable in editors that don't decode JSON
283-
# escapes, friendly to peers running ``cat`` or ``Get-Content``) and
284-
# so the encoding pin is a real contract instead of a future hedge.
285-
#
286-
# ``Path.write_text`` without ``encoding=`` falls back to the system
287-
# locale codec (cp1252 / gb2312 / cp932 on Windows), which would
288-
# mis-encode non-ASCII bytes locally and produce a file a peer with
289-
# a different locale couldn't decode. The sibling integration-
290-
# catalog writer in ``integrations/catalog.py`` pins
291-
# ``encoding="utf-8"`` for the same reason.
292-
dest.write_text(
293-
json.dumps(options, indent=2, sort_keys=True, ensure_ascii=False),
294-
encoding="utf-8",
295-
)
296-
297-
298-
def load_init_options(project_path: Path) -> dict[str, Any]:
299-
"""Load the init options previously saved by ``specify init``.
300-
301-
Returns an empty dict if the file does not exist or cannot be parsed.
302-
"""
303-
path = project_path / INIT_OPTIONS_FILE
304-
if not path.exists():
305-
return {}
306-
try:
307-
# Match the explicit UTF-8 used by ``save_init_options``; without
308-
# it ``read_text`` falls back to the system codec on Windows and
309-
# raises ``UnicodeDecodeError`` on any file containing the
310-
# multi-byte UTF-8 sequences ``save_init_options`` now writes
311-
# directly. ``UnicodeDecodeError`` is a subclass of
312-
# ``ValueError``, not ``OSError`` / ``json.JSONDecodeError``, so
313-
# it must be listed explicitly here to preserve the existing
314-
# "fall back to empty dict" contract for corrupted / foreign-
315-
# codec files.
316-
return json.loads(path.read_text(encoding="utf-8"))
317-
except (json.JSONDecodeError, OSError, UnicodeDecodeError):
318-
return {}
319-
320-
321268
# ---------------------------------------------------------------------------
322269
# Agent-context extension config helpers
323270
# ---------------------------------------------------------------------------
@@ -401,10 +348,10 @@ def resolve_active_skills_dir(project_root: Path) -> Path | None:
401348
"""Return the active skills directory, creating it on demand when enabled.
402349
403350
Reads ``.specify/init-options.json`` to determine whether skills are
404-
enabled and which agent was selected. When ``ai_skills`` is true the
405-
directory is created safely (symlink/containment checks); when false
406-
only Kimi's native-skills fallback is honoured (directory must already
407-
exist).
351+
enabled and which agent was selected. Only ``ai_skills`` set to boolean
352+
``True`` creates the directory safely (symlink/containment checks); when
353+
``ai_skills`` is not boolean ``True``, only Kimi's native-skills fallback
354+
is honoured, and the native skills directory must already exist.
408355
409356
Returns:
410357
The skills directory ``Path``, or ``None`` if skills are not active.
@@ -425,14 +372,15 @@ def resolve_active_skills_dir(project_root: Path) -> Path | None:
425372
if not isinstance(agent, str) or not agent:
426373
return None
427374

428-
ai_skills_enabled = bool(opts.get("ai_skills"))
375+
ai_skills_enabled = _is_ai_skills_enabled(opts)
429376
if not ai_skills_enabled and agent != "kimi":
430377
return None
431378

432379
skills_dir = _get_skills_dir(project_root, agent)
433380

434381
if not ai_skills_enabled:
435-
# Kimi native-skills fallback: use the directory only if it exists.
382+
# Kimi native-skills fallback when ai_skills is not boolean True:
383+
# use the native skills directory only if it already exists.
436384
if not skills_dir.is_dir():
437385
return None
438386
_ensure_safe_shared_directory(
@@ -441,7 +389,7 @@ def resolve_active_skills_dir(project_root: Path) -> Path | None:
441389
)
442390
return skills_dir
443391

444-
# ai_skills is explicitly enabled — create the directory safely.
392+
# ai_skills is boolean True: create the directory safely.
445393
_ensure_safe_shared_directory(
446394
project_root, skills_dir, context="agent skills directory",
447395
)

src/specify_cli/_init_options.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
"""Helpers for interpreting persisted init options."""
2+
3+
import json
4+
from collections.abc import Mapping
5+
from pathlib import Path
6+
from typing import Any
7+
8+
9+
INIT_OPTIONS_FILE = ".specify/init-options.json"
10+
11+
12+
def save_init_options(project_path: Path, options: dict[str, Any]) -> None:
13+
"""Persist the CLI options used during ``specify init``."""
14+
dest = project_path / INIT_OPTIONS_FILE
15+
dest.parent.mkdir(parents=True, exist_ok=True)
16+
dest.write_text(
17+
json.dumps(options, indent=2, sort_keys=True, ensure_ascii=False),
18+
encoding="utf-8",
19+
)
20+
21+
22+
def load_init_options(project_path: Path) -> dict[str, Any]:
23+
"""Load persisted init options, returning an empty dict when unavailable."""
24+
path = project_path / INIT_OPTIONS_FILE
25+
if not path.exists():
26+
return {}
27+
try:
28+
payload = json.loads(path.read_text(encoding="utf-8"))
29+
except (json.JSONDecodeError, OSError, UnicodeError):
30+
return {}
31+
return payload if isinstance(payload, dict) else {}
32+
33+
34+
def is_ai_skills_enabled(opts: Mapping[str, Any] | None) -> bool:
35+
"""Return True only when init options explicitly enable AI skills."""
36+
return isinstance(opts, Mapping) and opts.get("ai_skills") is True

src/specify_cli/agents.py

Lines changed: 96 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515

1616
import yaml
1717

18+
from ._init_options import is_ai_skills_enabled, load_init_options
19+
1820

1921
def _build_agent_configs() -> dict[str, Any]:
2022
"""Derive CommandRegistrar.AGENT_CONFIGS from INTEGRATION_REGISTRY."""
@@ -359,11 +361,6 @@ def resolve_skill_placeholders(
359361
agent_name: str, frontmatter: dict, body: str, project_root: Path
360362
) -> str:
361363
"""Resolve script placeholders for skills-backed agents."""
362-
try:
363-
from . import load_init_options
364-
except ImportError:
365-
return body
366-
367364
if not isinstance(frontmatter, dict):
368365
frontmatter = {}
369366

@@ -474,6 +471,29 @@ def _is_safe_command_name(name: str) -> bool:
474471
return False
475472
return os.path.normpath(name) == name
476473

474+
@staticmethod
475+
def _same_lexical_path(left: Path, right: Path) -> bool:
476+
"""Compare paths after lexical normalization without resolving symlinks."""
477+
return os.path.normcase(os.path.normpath(os.fspath(left))) == os.path.normcase(
478+
os.path.normpath(os.fspath(right))
479+
)
480+
481+
@staticmethod
482+
def _active_skills_agent(project_root: Path) -> Optional[str]:
483+
"""Return the initialized skills-backed agent, if skills mode is active."""
484+
opts = load_init_options(project_root)
485+
if not isinstance(opts, dict):
486+
return None
487+
488+
agent = opts.get("ai")
489+
if not isinstance(agent, str) or not agent:
490+
return None
491+
# Kimi is a native skills integration; when ai_skills is not boolean
492+
# True, Kimi still uses its existing SKILL.md layout.
493+
if not is_ai_skills_enabled(opts) and agent != "kimi":
494+
return None
495+
return agent
496+
477497
def register_commands(
478498
self,
479499
agent_name: str,
@@ -806,6 +826,7 @@ def register_commands_for_all_agents(
806826
project_root: Path,
807827
context_note: str = None,
808828
link_outputs: bool = False,
829+
create_missing_active_skills_dir: bool = False,
809830
) -> Dict[str, List[str]]:
810831
"""Register commands for all detected agents in the project.
811832
@@ -817,28 +838,85 @@ def register_commands_for_all_agents(
817838
context_note: Custom context comment for markdown output
818839
link_outputs: If True, create dev-mode symlinks for rendered
819840
command files when supported by the OS.
841+
create_missing_active_skills_dir: If True, attempt missing-dir
842+
recovery only for the active initialized skills-backed agent.
843+
Recovery requires active skills mode (or Kimi's existing native
844+
skills directory) and is skipped when safe resolution or
845+
creation fails.
820846
821847
Returns:
822848
Dictionary mapping agent names to list of registered commands
823849
"""
824850
results = {}
825851

826852
self._ensure_configs()
853+
active_skills_agent = (
854+
self._active_skills_agent(project_root)
855+
if create_missing_active_skills_dir else None
856+
)
857+
active_created_skills_dir: Optional[Path] = None
827858
for agent_name, agent_config in self.AGENT_CONFIGS.items():
859+
active_skills_output = (
860+
agent_name == active_skills_agent
861+
and agent_config.get("extension") == "/SKILL.md"
862+
)
863+
recovered_active_skills_dir: Optional[Path] = None
828864
# Check detect_dir first (project-local marker) if configured,
829865
# falling back to the resolved dir for output. This prevents
830866
# global dirs (e.g. ~/.hermes/skills) from causing false
831867
# detection in every project.
832868
detect_dir_str = agent_config.get("detect_dir")
833869
if detect_dir_str:
834870
detect_path = project_root / detect_dir_str
835-
if not detect_path.exists():
836-
continue
871+
if not detect_path.is_dir():
872+
if not active_skills_output:
873+
continue
874+
try:
875+
from . import resolve_active_skills_dir
876+
877+
recovered_active_skills_dir = (
878+
resolve_active_skills_dir(project_root)
879+
)
880+
except (ValueError, OSError):
881+
continue
882+
if recovered_active_skills_dir is None or not detect_path.is_dir():
883+
continue
884+
active_created_skills_dir = recovered_active_skills_dir
837885
agent_dir = self._resolve_agent_dir(
838886
agent_name, agent_config, project_root,
839887
)
840888

841-
if agent_dir.exists():
889+
agent_dir_existed = agent_dir.is_dir()
890+
register_missing_active_skills_agent = (
891+
not agent_dir_existed
892+
and active_skills_output
893+
)
894+
if register_missing_active_skills_agent:
895+
if recovered_active_skills_dir is None:
896+
try:
897+
from . import resolve_active_skills_dir
898+
899+
recovered_active_skills_dir = (
900+
resolve_active_skills_dir(project_root)
901+
)
902+
except (ValueError, OSError):
903+
continue
904+
if recovered_active_skills_dir is None:
905+
continue
906+
active_created_skills_dir = recovered_active_skills_dir
907+
# Shared skill dirs such as .agents/skills should not make
908+
# later integrations look detected when the active agent just
909+
# recreated the directory during this registration pass.
910+
created_by_active_agent = (
911+
active_created_skills_dir is not None
912+
and self._same_lexical_path(agent_dir, active_created_skills_dir)
913+
and agent_name != active_skills_agent
914+
)
915+
should_register = (
916+
agent_dir_existed and not created_by_active_agent
917+
) or register_missing_active_skills_agent
918+
919+
if should_register:
842920
try:
843921
registered = self.register_commands(
844922
agent_name,
@@ -852,8 +930,16 @@ def register_commands_for_all_agents(
852930
)
853931
if registered:
854932
results[agent_name] = registered
933+
if register_missing_active_skills_agent:
934+
active_created_skills_dir = (
935+
recovered_active_skills_dir or agent_dir
936+
)
855937
except ValueError:
856938
continue
939+
except OSError:
940+
if register_missing_active_skills_agent:
941+
continue
942+
raise
857943

858944
return results
859945

@@ -892,12 +978,12 @@ def register_commands_for_non_skill_agents(
892978
detect_dir_str = agent_config.get("detect_dir")
893979
if detect_dir_str:
894980
detect_path = project_root / detect_dir_str
895-
if not detect_path.exists():
981+
if not detect_path.is_dir():
896982
continue
897983
agent_dir = self._resolve_agent_dir(
898984
agent_name, agent_config, project_root,
899985
)
900-
if agent_dir.exists():
986+
if agent_dir.is_dir():
901987
try:
902988
registered = self.register_commands(
903989
agent_name,

0 commit comments

Comments
 (0)