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. It also includes review follow-up fixes in the moved extension command code so the refactor branch stays current with reviewer feedback instead of carrying known issues forward.

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.
Rich markup hardening — user-controlled extension/catalog names and URL values rendered in Rich status/error output are escaped before display.
URL download path hardening — extension add <name> --from <zip-url> now writes downloaded ZIPs to a generated tempfile under .specify/extensions/.cache/downloads/, so the extension argument cannot influence the ZIP path.
Moved-code follow-ups — skill registration imports _print_cli_warning from the package root after the move, and catalog config writes use yaml.safe_dump.

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.
tests/test_extensions.py covers Rich markup escaping and the generated tempfile path for extension add --from downloads.

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

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

Please address Copilot feedback

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

Copilot

Copilot's findings

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

Copilot

Copilot's findings

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

mnriem · 2026-06-19T18:40:30Z

Please address Copilot feedback and resolve conflicts by rebasing on upstream/main

darion-yaphet · 2026-06-20T06:50:16Z

We urgently need a review tool. The review function provided by Copilot is too inefficient.

mnriem · 2026-06-22T16:19:05Z

Please address Copilot feedback

Copilot

Copilot's findings

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

mnriem · 2026-06-22T16:39:20Z

Please address Copilot feedback. If not applicable, please explain why

darion-yaphet · 2026-06-22T16:46:55Z

Please address Copilot feedback. If not applicable, please explain why

Getting entangled in trivial matters that don't matter at all and wasting time

Copilot

Copilot's findings

Files reviewed: 6/6 changed files
Comments generated: 1

mnriem · 2026-06-22T17:05:52Z

Please address Copilot feedback

Copilot

Copilot's findings

Files reviewed: 6/6 changed files
Comments generated: 0 new

…(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.

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.

…stration register_enabled_extensions_for_agent imported _print_cli_warning from `.` (the extensions package), but the helper lives in the parent specify_cli package. The wrong level raised ImportError inside the error handlers, aborting extension/skill registration on the first failure instead of warning and continuing. Use `..` to match the other parent-package imports.

User-provided arguments and extension/catalog metadata (names, descriptions, versions, IDs, paths) were interpolated into Rich markup strings without escaping. Values containing markup sequences (e.g. [red]...) would be parsed as markup, allowing output injection that could corrupt or mislead CLI messages. Wrap all such interpolations with rich.markup.escape across the extension/catalog command handlers: list, search, info (_print_extension_info), add (including --dev paths), remove, enable, disable, set-priority, update, and the ambiguous-match resolvers (error strings and Table rows). Reuse the already-computed safe_extension where available. Escaping is a no-op for benign strings, so normal output is unchanged.

User-controlled catalog URLs and extension IDs are rendered through Rich-enabled console paths, so every remaining output-only interpolation now escapes markup while leaving stored values and filesystem behavior unchanged. Regression tests cover catalog add, install hints, remove hints, and state command messages with bracketed markup-like values.

Rich markup remains enabled for styled CLI messages, so exception text and config path labels must be escaped before rendering. YAML parser errors, URL validation failures, download errors, and extension validation errors can include user-controlled catalog or manifest values. Constraint: Preserve existing exception handling and user-facing error paths Rejected: Disable Rich markup for these messages | existing output intentionally uses markup for labels and styling Confidence: high Scope-risk: narrow Directive: Escape user-controlled exception text before interpolating into Rich-rendered strings Tested: .venv/bin/python -m pytest tests/test_extensions.py -q Co-authored-by: OmX <omx@oh-my-codex.dev>

Catalog path labels are rendered through Rich markup and downloaded update manifests are trusted long enough to validate extension IDs. Escape displayed project paths before rendering, and reject non-mapping extension.yml payloads before ID validation so bad archives fail with a clear rollback reason.

Copilot

Copilot's findings

Files reviewed: 6/6 changed files
Comments generated: 0 new

mnriem · 2026-06-22T18:41:03Z

Thank you!

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 Outdated

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

Comment thread src/specify_cli/extensions/_commands.py Outdated

Comment thread src/specify_cli/extensions/_commands.py Outdated

Comment thread src/specify_cli/extensions/_commands.py Outdated

Comment thread src/specify_cli/extensions/_commands.py Outdated

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

Open

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

Comment thread tests/test_extension_update_hardening.py

Comment thread src/specify_cli/extensions/_commands.py

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

mnriem requested a review from Copilot June 19, 2026 15:21

Copilot started reviewing on behalf of mnriem June 19, 2026 15:21 View session

Copilot AI reviewed Jun 19, 2026

View reviewed changes

Comment thread src/specify_cli/extensions/_commands.py Outdated

Comment thread src/specify_cli/extensions/_commands.py

Comment thread src/specify_cli/extensions/_commands.py

mnriem requested a review from Copilot June 19, 2026 17:33

Copilot started reviewing on behalf of mnriem June 19, 2026 17:34 View session

Copilot AI reviewed Jun 19, 2026

View reviewed changes

Comment thread src/specify_cli/extensions/_commands.py Outdated

Comment thread src/specify_cli/extensions/_commands.py Outdated

darion-yaphet force-pushed the refactor/split-init-pr7 branch from 19dfad7 to 9f9e9e6 Compare June 20, 2026 07:11

mnriem requested a review from Copilot June 22, 2026 12:37

mnriem requested a review from Copilot June 22, 2026 16:33

Copilot started reviewing on behalf of mnriem June 22, 2026 16:33 View session

Copilot AI reviewed Jun 22, 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 a review from Copilot June 22, 2026 16:59

Copilot started reviewing on behalf of mnriem June 22, 2026 16:59 View session

Copilot AI reviewed Jun 22, 2026

View reviewed changes

Comment thread src/specify_cli/extensions/_commands.py Outdated

darion-yaphet force-pushed the refactor/split-init-pr7 branch from d080d5c to 6782384 Compare June 22, 2026 17:13

mnriem requested a review from Copilot June 22, 2026 17:15

Copilot started reviewing on behalf of mnriem June 22, 2026 17:15 View session

Copilot AI reviewed Jun 22, 2026

View reviewed changes

darion-yaphet and others added 8 commits June 23, 2026 01:23

darion-yaphet force-pushed the refactor/split-init-pr7 branch from 6782384 to 3711804 Compare June 22, 2026 17:30

mnriem requested a review from Copilot June 22, 2026 18:03

Copilot started reviewing on behalf of mnriem June 22, 2026 18:03 View session

Copilot AI reviewed Jun 22, 2026

View reviewed changes

mnriem self-requested a review June 22, 2026 18:38

mnriem approved these changes Jun 22, 2026

View reviewed changes

mnriem merged commit 826e193 into github:main Jun 22, 2026
11 checks passed

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!

Uh oh!

Uh oh!

Uh oh!

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!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

mnriem commented Jun 19, 2026

Uh oh!

darion-yaphet commented Jun 20, 2026

Uh oh!

mnriem commented Jun 22, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mnriem commented Jun 22, 2026

Uh oh!

darion-yaphet commented Jun 22, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

Uh oh!

mnriem commented Jun 22, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

darion-yaphet commented Jun 17, 2026 •

edited

Loading