Skip to content

Commit 7e1b5ec

Browse files
committed
fix: address PR review — lazy init, assertions, deprecated flags
- _ensure_configs(): catch ImportError (not Exception), don't set _configs_loaded on failure so retries work - Move _ensure_configs() before unregister loop (not inside it) - Module-level try/except catches ImportError specifically - Remove tautology assertion (or True) in test_extensions.py - Strengthen preset provenance assertion to check source: field - Mark --offline, --skip-tls, --debug, --github-token as hidden deprecated no-ops in init() 1086 tests pass.
1 parent bdcd334 commit 7e1b5ec

4 files changed

Lines changed: 15 additions & 11 deletions

File tree

src/specify_cli/__init__.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -851,11 +851,11 @@ def init(
851851
no_git: bool = typer.Option(False, "--no-git", help="Skip git repository initialization"),
852852
here: bool = typer.Option(False, "--here", help="Initialize project in the current directory instead of creating a new one"),
853853
force: bool = typer.Option(False, "--force", help="Force merge/overwrite when using --here (skip confirmation)"),
854-
skip_tls: bool = typer.Option(False, "--skip-tls", help="Skip SSL/TLS verification (not recommended)"),
855-
debug: bool = typer.Option(False, "--debug", help="Show verbose diagnostic output for network and extraction failures"),
856-
github_token: str = typer.Option(None, "--github-token", help="GitHub token to use for API requests (or set GH_TOKEN or GITHUB_TOKEN environment variable)"),
854+
skip_tls: bool = typer.Option(False, "--skip-tls", help="Deprecated (no-op). Previously: skip SSL/TLS verification.", hidden=True),
855+
debug: bool = typer.Option(False, "--debug", help="Deprecated (no-op). Previously: show verbose diagnostic output.", hidden=True),
856+
github_token: str = typer.Option(None, "--github-token", help="Deprecated (no-op). Previously: GitHub token for API requests.", hidden=True),
857857
ai_skills: bool = typer.Option(False, "--ai-skills", help="Install Prompt.MD templates as agent skills (requires --ai)"),
858-
offline: bool = typer.Option(False, "--offline", help="Use assets bundled in the specify-cli package instead of downloading from GitHub (no network access required). Bundled assets will become the default in v0.6.0 and this flag will be removed."),
858+
offline: bool = typer.Option(False, "--offline", help="Deprecated (no-op). All scaffolding now uses bundled assets.", hidden=True),
859859
preset: str = typer.Option(None, "--preset", help="Install a preset during initialization (by preset ID)"),
860860
branch_numbering: str = typer.Option(None, "--branch-numbering", help="Branch numbering strategy: 'sequential' (001, 002, ...) or 'timestamp' (YYYYMMDD-HHMMSS)"),
861861
integration: str = typer.Option(None, "--integration", help="Use the new integration system (e.g. --integration copilot). Mutually exclusive with --ai."),

src/specify_cli/agents.py

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,11 @@ def __init_subclass__(cls, **kwargs: Any) -> None:
5050
@classmethod
5151
def _ensure_configs(cls) -> None:
5252
if not cls._configs_loaded:
53-
cls._configs_loaded = True
54-
cls.AGENT_CONFIGS = _build_agent_configs()
53+
try:
54+
cls.AGENT_CONFIGS = _build_agent_configs()
55+
cls._configs_loaded = True
56+
except ImportError:
57+
pass # Circular import during module init; retry on next access
5558

5659
@staticmethod
5760
def parse_frontmatter(content: str) -> tuple[dict, str]:
@@ -510,8 +513,8 @@ def unregister_commands(
510513
registered_commands: Dict mapping agent names to command name lists
511514
project_root: Path to project root
512515
"""
516+
self._ensure_configs()
513517
for agent_name, cmd_names in registered_commands.items():
514-
self._ensure_configs()
515518
if agent_name not in self.AGENT_CONFIGS:
516519
continue
517520

@@ -531,9 +534,10 @@ def unregister_commands(
531534

532535

533536
# Populate AGENT_CONFIGS after class definition.
534-
# The deferred import avoids circular import issues during module loading.
537+
# Catches ImportError from circular imports during module loading;
538+
# _configs_loaded stays False so the next explicit access retries.
535539
try:
536540
CommandRegistrar._ensure_configs()
537-
except Exception:
538-
pass # Silently defer to first explicit access
541+
except ImportError:
542+
pass
539543

tests/test_extensions.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1040,7 +1040,6 @@ def test_register_commands_for_claude(self, extension_dir, project_dir):
10401040
content = cmd_file.read_text()
10411041
assert "description: Test hello command" in content
10421042
assert "test-ext" in content
1043-
assert "test-ext" in content or True # skill format uses metadata.source instead
10441043

10451044
def test_command_with_aliases(self, project_dir, temp_dir):
10461045
"""Test registering a command with aliases."""

tests/test_presets.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1785,6 +1785,7 @@ def test_self_test_registers_commands_for_claude(self, project_dir):
17851785
assert cmd_file.exists(), "Skill not registered in .claude/skills/"
17861786
content = cmd_file.read_text()
17871787
assert "self-test" in content
1788+
assert "source:" in content # skill frontmatter includes metadata.source
17881789

17891790
def test_self_test_registers_commands_for_gemini(self, project_dir):
17901791
"""Test that installing self-test registers commands in .gemini/commands/ as TOML."""

0 commit comments

Comments
 (0)