refactor: move extension command handlers to extensions/_commands.py (PR-7/8) by darion-yaphet · Pull Request #3014 · github/spec-kit

darion-yaphet · 2026-06-17T02:41:49Z

Summary

PR-7 of the 8-part effort to break up the specify_cli/__init__.py monolith. Continues the domain-dir layout established in PR-5 (integrations/_commands.py) and PR-6 (presets/_commands.py).

This PR moves the extension * and catalog * command handlers out of __init__.py. Because extensions was a flat module (unlike the already-package presets/integrations), it is first converted to a package.

This is primarily a structural refactor, with one explicit bug fix in the moved code: skills-based extension update rollback now preserves each SKILL.md under its per-command subdirectory instead of allowing backup path collisions.

Changes

extensions.py → extensions/__init__.py — pure rename (99% identical). The module's intra-file relative imports are bumped from .x to ..x, since they reference root-package siblings (.._init_options, ..catalogs, ..agents, ..integrations, and from .. import for load_init_options, _print_cli_warning, AGENT_CONFIG, DEFAULT_SKILLS_DIR, _get_skills_dir).
New extensions/_commands.py — holds extension_app, catalog_app, all 12 command handlers (list/add/remove/search/info/update/enable/disable/set-priority + catalog list/add/remove), the private helpers (_resolve_installed_extension, _resolve_catalog_extension, _print_extension_info), and a register(app) entry point.
__init__.py drops ~1444 lines (3511 → 2067). The extension command group is re-attached via register(app), preserving the CLI surface.
Skills rollback hardening — extension update backup paths now include the per-command skill subdirectory so multiple skills named SKILL.md restore independently during rollback.

Monkeypatch compatibility

Root helpers (_require_specify_project, _locate_bundled_extension, load_init_options, _display_project_path) are reached through thin shims in _commands.py that re-fetch from the parent package at call time. This keeps existing specify_cli.<helper> monkeypatch targets working with no test changes.

Verification

extension/catalog command tree loads and all --help surfaces respond.
Full test suite failure set is identical before and after this change (82 pre-existing environment failures unrelated to the refactor; 0 new, 0 fixed-by-accident).
tests/test_extension_update_hardening.py covers corrupted update config rollback and the skills SKILL.md backup collision regression.

Copilot

Pull request overview

Refactors the Specify CLI by extracting specify extension * and specify extension catalog * command handlers out of the specify_cli/__init__.py monolith into a dedicated extensions/_commands.py module, while converting extensions from a flat module into a package to support the domain-dir layout.

Changes:

Introduces src/specify_cli/extensions/_commands.py containing the Typer apps (extension, catalog) and all related command handlers, plus a register(app) entry point.
Converts extensions.py into extensions/__init__.py and updates intra-package imports accordingly.
Re-attaches the extension command group from specify_cli/__init__.py via _register_extension_cmds(app) to preserve the CLI surface.

Show a summary per file

File	Description
src/specify_cli/extensions/_commands.py	New home for `extension` / `catalog` Typer apps and all extension-related CLI handlers (registered via `register(app)`).
src/specify_cli/extensions/init.py	Converts `extensions` into a package and adjusts relative imports to continue referencing root-package siblings.
src/specify_cli/init.py	Removes inline extension command definitions and registers the new extensions command module to keep the same CLI entry points.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Files reviewed: 3/3 changed files
Comments generated: 3

mnriem

Please address Copilot feedback and resolve conflicts

darion-yaphet · 2026-06-17T13:55:17Z

fixed

Copilot

Copilot's findings

Files reviewed: 3/3 changed files
Comments generated: 4

+                        output_name = _AgentReg._compute_output_name(agent_name, cmd_name, agent_config)
+                        cmd_file = commands_dir / f"{output_name}{agent_config['extension']}"
+                        if cmd_file.exists():
+                            backup_cmd_path = backup_commands_dir / agent_name / cmd_file.name


+    })
+
+    config["catalogs"] = catalogs
+    config_path.write_text(yaml.dump(config, default_flow_style=False, sort_keys=False, allow_unicode=True), encoding="utf-8")


darion-yaphet · 2026-06-18T03:40:22Z

+        raise typer.Exit(1)
+
+    config["catalogs"] = catalogs
+    config_path.write_text(yaml.dump(config, default_flow_style=False, sort_keys=False, allow_unicode=True), encoding="utf-8")


The principle is sound but doesn't apply here — no bug, no behavioral change, pure code hygiene.

yaml.dump only differs from yaml.safe_dump when it encounters non-basic Python types (custom objects, set, tuple, etc.): the default Dumper emits !!python/object tags
for them, whereas SafeDumper raises instead. That's the real safety argument for safe_dump.

But the data written at this call site is built entirely from basic types — str, int, bool, dict, list:

{"name": str, "url": str, "priority": int,
"install_allowed": bool, "description": str}

For input like this, yaml.dump and yaml.safe_dump produce byte-for-byte identical output — no tags are ever emitted, because there are no non-basic types to tag. So
switching to safe_dump here changes nothing observable: no bug fixed, no output difference, no edge case closed.

It's purely a defensive/consistency nicety, not a correctness fix — and given the project already uses yaml.dump in most places, "consistency" cuts both ways.

darion-yaphet · 2026-06-18T03:53:16Z

+    # Default (no flags) lists installed; --all also lists installed.
+    # --available alone lists only catalog extensions, not installed.
+    show_installed = all_extensions or not available
+    show_available = available or all_extensions
+


You're right — this is a genuine behavior change and doesn't belong in a "pure structural move" PR. Thanks for catching it.

For context on why the change exists: the --available / --all flags aren't new. They've existed since the original extension system (#1551, f14a47e), and their help
text has always advertised:

--available: "Show available extensions from catalog"

--all: "Show both installed and available"

But the implementation from day one was a stub — passing either flag just printed a static Install an extension: specify extension add hint and never queried
the catalog. The ExtensionCatalog infrastructure was already present in that same #1551 PR; the flags were simply never wired to it. So commit 8c28922 makes the flags
finally do what their help text and the docs have claimed all along.

In other words this is a long-standing dead/misleading-flag fix, not a new feature — but you're correct that it's orthogonal to the refactor and should not ride along
silently under "no behavior change."

Proposed fix: I'll pull 8c28922 out of this refactor PR and ship it separately, with test coverage for list --available / --all (catalog query + installed-ID
filtering + the catalog-unavailable error path). Note that commit also contains an unrelated path-traversal sanitization fix for extension add --from, so I'll split
that into its own change too rather than dropping it. This refactor PR will then be a true structural-only move.

mnriem · 2026-06-17T15:25:08Z

Please address Copilot feedback

…(PR-7/8) Convert the flat extensions.py module into an extensions/ package and extract all extension_app and catalog_app command handlers plus their private helpers (_resolve_installed_extension, _resolve_catalog_extension, _print_extension_info) out of __init__.py into the new extensions/_commands.py, mirroring the domain-dir layout used for presets/_commands.py (PR-6) and integrations/_commands.py (PR-5). - extensions.py -> extensions/__init__.py (pure rename, 99%); intra-module relative imports bumped from `.x` to `..x` since they reference root siblings. - Root helpers (_require_specify_project, _locate_bundled_extension, load_init_options, _display_project_path) are reached through thin shims that re-fetch from the parent package at call time, so test monkeypatching of specify_cli.<helper> keeps working unchanged. - __init__.py drops ~1444 lines (3511 -> 2067); CLI surface preserved via register(app). No behavior change. Full suite failure set is identical before/after (82 pre-existing env failures, 0 new).

…s agents Skills agents (extension == "/SKILL.md") name every command file SKILL.md, each in its own per-command subdir (e.g. speckit-plan/SKILL.md). The update backup keyed the backup path on cmd_file.name alone, so all of an agent's skill files collided onto a single backup path — each shutil.copy2 overwrote the previous one, and rollback restored one skill's content over all the others, corrupting or losing the rest. Mirror the real on-disk layout by using cmd_file.relative_to(commands_dir), keeping each backup path unique. This also makes backed_up_command_files values unique so restore copies the correct content back to each command. Add a regression test asserting two distinct skill files survive a backup -> failed-update -> rollback cycle with their own content.

darion-yaphet · 2026-06-18T05:22:34Z

Heads-up on merge ordering

I force-pushed this branch to remove the extension list --available behavior change (and the bundled add --from path-traversal fix) that had been riding along here. This PR is now a structural-only move — no behavior changes.

The extracted change now lives in its own PR, #3051, with test coverage.

Merge order matters, because #3051 is stacked on this branch:

Merge this PR (refactor: move extension command handlers to extensions/_commands.py (PR-7/8) #3014) first. It introduces extensions/_commands.py.
Then fix(extensions): wire up list --available catalog query + harden add --from path traversal #3051. It edits extensions/_commands.py, so it can't apply until this lands. Until then, fix(extensions): wire up list --available catalog query + harden add --from path traversal #3051's diff temporarily includes this PR's structural-move commit; once this merges into main, that commit drops out of fix(extensions): wire up list --available catalog query + harden add --from path traversal #3051's diff, leaving only the fix + tests.

If #3051 is reviewed before this merges, please ignore the structural-move commit in its diff — it's this PR, and will disappear on rebase.

Copilot

Copilot's findings

Files reviewed: 4/4 changed files
Comments generated: 2

+    """Regression: skills agents name every command file SKILL.md (one per
+    per-command subdir). Backup must keep the per-command path so rollback
+    restores each skill's own content instead of overwriting them onto a


The catalog add/remove handlers wrote the integration catalog config with yaml.dump. Switch to yaml.safe_dump to align with the SafeDumper used by the presets commands and to refuse emitting !!python/object tags if a non-basic value ever reaches the config dict. Output is unchanged for the current basic-type payload (str/int/bool/dict/ list) — this is a defensive/consistency change, not a behavioral fix.

darion-yaphet requested a review from mnriem as a code owner June 17, 2026 02:41

darion-yaphet force-pushed the refactor/split-init-pr7 branch from 5da1bab to 29be5d8 Compare June 17, 2026 02:48

mnriem requested a review from Copilot June 17, 2026 12:27

Copilot started reviewing on behalf of mnriem June 17, 2026 12:28 View session

Copilot AI reviewed Jun 17, 2026

View reviewed changes

Comment thread src/specify_cli/extensions/_commands.py

Comment thread src/specify_cli/extensions/_commands.py

Comment thread src/specify_cli/extensions/_commands.py

mnriem requested changes Jun 17, 2026

View reviewed changes

darion-yaphet force-pushed the refactor/split-init-pr7 branch from 50756f8 to ddcc165 Compare June 17, 2026 13:54

mnriem requested a review from Copilot June 17, 2026 14:37

Copilot started reviewing on behalf of mnriem June 17, 2026 14:37 View session

Copilot AI reviewed Jun 17, 2026

View reviewed changes

darion-yaphet added 2 commits June 18, 2026 10:50

darion-yaphet force-pushed the refactor/split-init-pr7 branch from ddcc165 to efe0633 Compare June 18, 2026 05:08

This was referenced Jun 18, 2026

fix(extensions): wire up list --available catalog query + harden add --from path traversal darion-yaphet/spec-kit#2

Closed

fix(extensions): wire up list --available catalog query + harden add --from path traversal #3051

Draft

mnriem requested a review from Copilot June 18, 2026 13:33

Copilot started reviewing on behalf of mnriem June 18, 2026 13:33 View session

Copilot AI reviewed Jun 18, 2026

View reviewed changes

darion-yaphet force-pushed the refactor/split-init-pr7 branch from efe0633 to 2d13763 Compare June 18, 2026 14:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: move extension command handlers to extensions/_commands.py (PR-7/8)#3014

refactor: move extension command handlers to extensions/_commands.py (PR-7/8)#3014
darion-yaphet wants to merge 3 commits into
github:mainfrom
darion-yaphet:refactor/split-init-pr7

darion-yaphet commented Jun 17, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mnriem left a comment

Uh oh!

darion-yaphet commented Jun 17, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

darion-yaphet Jun 18, 2026

Uh oh!

darion-yaphet Jun 18, 2026

Uh oh!

mnriem commented Jun 17, 2026

Uh oh!

darion-yaphet commented Jun 18, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

darion-yaphet commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Monkeypatch compatibility

Verification

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mnriem left a comment

Choose a reason for hiding this comment

Uh oh!

darion-yaphet commented Jun 17, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

darion-yaphet Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

darion-yaphet Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

mnriem commented Jun 17, 2026

Uh oh!

darion-yaphet commented Jun 18, 2026

Heads-up on merge ordering

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

darion-yaphet commented Jun 17, 2026 •

edited

Loading