Skip to content

Refactor extension CLI command structure - #4641

Open
mnriem wants to merge 2 commits into
github:mainfrom
mnriem:mnriem-refactor-extension-list
Open

mnriem wants to merge 2 commits into
github:mainfrom
mnriem:mnriem-refactor-extension-list

Conversation

@mnriem

@mnriem mnriem commented Sep 19, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • split each specify extension command into a dedicated command_*.py module
  • mirror the nested extension catalog command surface in an extensions/catalog/ package
  • keep command_update.py as the registered adapter while separating discovery, artifact validation, and transactional behavior into private phase modules
  • reorganize command tests under tests/specify_cli/extensions/ to mirror the implementation
  • document the CLI architecture and rationale in design/cli.md, linked from AGENTS.md

Validation

  • complete repository test suite passed
  • final affected command, extension-domain, and integration CLI suites: 714 passed
  • Ruff and git diff --check passed
  • wheel build succeeded and contains all newly introduced modules
  • all top-level extension commands register exactly once, with the nested catalog group present

AI disclosure

This pull request and its implementation were fully authored on behalf of @mnriem by GitHub Copilot (model: GPT-5.6 Sol, autonomous). The code, tests, design documentation, commit message, and pull request description are AI-drafted.

Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings September 19, 2026 14:18
Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@mnriem

mnriem commented Sep 19, 2026

Copy link
Copy Markdown
Collaborator Author

The failing check was markdownlint, not Ruff. I removed the extra blank line at EOF in design/cli.md and reproduced the CI git diff --check successfully. Fix commit: 66ddfb14.

Posted on behalf of @mnriem by GitHub Copilot (model: GPT-5.6 Sol, autonomous); comment fully AI-drafted.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Update transactions contain security, rollback, malformed-version, and partial-success consistency defects.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 High severity · 4 Medium severity · 1 Low severity

Open (6)
What changed in this PR

Refactors extension CLI commands into predictable modules, introduces dedicated update phases, mirrors tests, and documents the architecture.

Changes:

  • Split extension and catalog commands into dedicated modules.
  • Separated update discovery, artifact validation, and transaction logic.
  • Reorganized tests and added architectural guidance.
File Description
AGENTS.md Links CLI architecture guidance.
design/​cli.md Documents command structure and conventions.
src/​specify_cli/​extensions/​__init__.py Documents package layout.
src/​specify_cli/​extensions/​command_add.py Implements extension installation.
src/​specify_cli/​extensions/​command_disable.py Implements disabling extensions.
src/​specify_cli/​extensions/​command_enable.py Implements enabling extensions.
src/​specify_cli/​extensions/​command_info.py Implements extension details.
src/​specify_cli/​extensions/​command_list.py Implements extension listing.
src/​specify_cli/​extensions/​command_remove.py Implements extension removal.
src/​specify_cli/​extensions/​command_search.py Implements catalog searching.
src/​specify_cli/​extensions/​command_set_priority.py Implements priority changes.
src/​specify_cli/​extensions/​command_update.py Registers the update adapter.
src/​specify_cli/​extensions/​_command_update_artifacts.py Preflights update archives.
src/​specify_cli/​extensions/​_command_update_discovery.py Discovers update candidates.
src/​specify_cli/​extensions/​_command_update_transaction.py Handles update transactions and rollback.
src/​specify_cli/​extensions/​catalog/​__init__.py Registers the catalog subgroup.
src/​specify_cli/​extensions/​catalog/​_helpers.py Loads catalog configuration.
src/​specify_cli/​extensions/​catalog/​command_add.py Implements catalog addition.
src/​specify_cli/​extensions/​catalog/​command_list.py Implements catalog listing.
src/​specify_cli/​extensions/​catalog/​command_remove.py Implements catalog removal.
tests/​specify_cli/​__init__.py Defines mirrored test package.
tests/​specify_cli/​extensions/​__init__.py Documents extension test layout.
tests/​specify_cli/​extensions/​_helpers.py Shares extension test utilities.
tests/​specify_cli/​extensions/​conftest.py Provides shared fixtures.
tests/​specify_cli/​extensions/​test_command_add.py Tests extension installation.
tests/​specify_cli/​extensions/​test_command_add_path_traversal.py Tests download path security.
tests/​specify_cli/​extensions/​test_command_disable.py Tests disabling extensions.
tests/​specify_cli/​extensions/​test_command_enable.py Tests enabling extensions.
tests/​specify_cli/​extensions/​test_command_info.py Tests extension details.
tests/​specify_cli/​extensions/​test_command_list.py Tests extension listing.
tests/​specify_cli/​extensions/​test_command_remove.py Tests extension removal.
tests/​specify_cli/​extensions/​test_command_search.py Tests catalog searching.
tests/​specify_cli/​extensions/​test_command_set_priority.py Tests priority changes.
tests/​specify_cli/​extensions/​test_command_update.py Tests public update behavior.
tests/​specify_cli/​extensions/​test_command_update_artifacts.py Tests update artifact handling.
tests/​specify_cli/​extensions/​test_command_update_discovery.py Tests update discovery.
tests/​specify_cli/​extensions/​test_command_update_transaction.py Tests update rollback and security.
tests/​specify_cli/​extensions/​catalog/​__init__.py Defines catalog test package.
tests/​specify_cli/​extensions/​catalog/​test_command_add.py Tests catalog addition.
tests/​specify_cli/​extensions/​catalog/​test_command_list.py Tests catalog listing.
tests/​specify_cli/​extensions/​catalog/​test_command_remove.py Tests catalog removal.
tests/​integrations/​test_cli.py Removes relocated catalog tests.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +526 to +530
_ = manager.install_from_zip(
archive_path,
speckit_version,
catalog_name=update.catalog_name,
)
Comment on lines +77 to +79
try:
installed_version = pkg_version.Version(metadata["version"])
except pkg_version.InvalidVersion:
Comment on lines +100 to +102
try:
catalog_version = pkg_version.Version(ext_info["version"])
except pkg_version.InvalidVersion:
Comment on lines +615 to +616
except KeyboardInterrupt:
raise
Comment on lines +853 to +864
if failed_updates:
console.print(f"[red]✗[/red] Failed to update {len(failed_updates)} extension(s):")
for ext_name, error in failed_updates:
console.print(f" • {_escape_markup(str(ext_name))}: {_escape_markup(str(error))}")
raise typer.Exit(1)

# S4: regenerate native event config after a successful update. An
# update replaces the installed extension.yml, so any added/removed/
# changed event declarations would otherwise leave native configs
# stale until a manual integration upgrade.
if updated_extensions:
_commands._refresh_events_and_warn(project_root)
Comment on lines +280 to +281
if failed or not (deployed or skipped):
console.print("\n[yellow]⚠[/yellow] Configuration may be required")
Copilot AI review requested due to automatic review settings September 19, 2026 14:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Malformed version values can abort update discovery, and config-free installs incorrectly emit a configuration warning.

Review effort: Balanced
Findings: 1 High severity · 4 Medium severity · 1 Low severity

Open (6)

@mnriem mnriem added the triage-nice-to-have Verdict: evidence-backed fix or greenlit feature — land after review label Sep 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

triage-nice-to-have Verdict: evidence-backed fix or greenlit feature — land after review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants