Skip to content

fix: scaffold extension config when installing via bundler - #4285

Open
chelsealong wants to merge 2 commits into
github:mainfrom
chelsealong:fix/4283-bundle-install-scaffold-config
Open

fix: scaffold extension config when installing via bundler#4285
chelsealong wants to merge 2 commits into
github:mainfrom
chelsealong:fix/4283-bundle-install-scaffold-config

Conversation

@chelsealong

Copy link
Copy Markdown
Contributor

Summary

Fixes #4283.

specify bundle install installs an extension by calling
ExtensionManager.install_from_directory / install_from_zip directly
from _ExtensionKindManager._do_install in
src/specify_cli/bundler/services/primitives.py, but never called
ExtensionManager.scaffold_config afterwards. The specify extension add
command flow (src/specify_cli/extensions/_commands.py) does call it, so
an extension installed as part of a bundle ended up with its
provides.config templates present on disk but never deployed — the
extension silently had no configuration, while installing the same
extension directly worked correctly.

Change

  • src/specify_cli/bundler/services/primitives.py: call
    self._manager.scaffold_config(manifest.id) right after a successful
    install_from_directory/install_from_zip call in
    _ExtensionKindManager._do_install, using the id from the manifest
    returned by the install call (matching what the extension add command
    flow does).
  • tests/unit/test_bundler_primitives.py: added
    test_bundled_extension_install_scaffolds_config, which installs a real
    (unmocked) extension source with a provides.config entry through
    primitive_manager("extensions", ...) and asserts the config file lands
    at .specify/extensions/<id>/<name>. Also updated the existing
    install_from_directory mocks in this file to return an object with an
    .id attribute (as the real method does), since the new call site reads
    manifest.id off the return value.

Out of scope: the issue's related documentation nit about
_target_follows_preserved_convention naming conventions not being
documented in EXTENSION-API-REFERENCE.md, and surfacing per-file
deployed/skipped/failed console reporting through the bundler's CLI
summary (the bundler currently reports install results only as install
counts, not per-component detail — plumbing that through is a larger,
separate change).

Test plan

Confirmed the new test fails without the fix (temporarily reverted just
the production file):

$ git stash push -- src/specify_cli/bundler/services/primitives.py
$ python3 -m uv run pytest tests/unit/test_bundler_primitives.py -q
...
FAILED tests/unit/test_bundler_primitives.py::test_bundled_extension_install_scaffolds_config - AssertionError: assert False
 +  where False = exists()
 +    where exists = PosixPath('.../project/.specify/extensions/my-ext/my-ext-config.yml').exists
1 failed, 20 passed in 0.28s
$ git stash pop

With the fix:

$ python3 -m uv run pytest tests/unit/test_bundler_primitives.py -q
21 passed in 0.27s

Full suite (unmodified elsewhere):

$ python3 -m uv run pytest -q
7309 passed, 10 skipped, 48 warnings in 467.66s

ruff check on the touched files reports 4 pre-existing issues (unused
noqa on lines untouched by this change, import-order in two unrelated
test functions) that are also present on unmodified upstream/main
confirmed by stashing this change and re-running the same ruff check
invocation, which reported the identical 4 findings at the same
locations.

AI disclosure

This change was implemented with AI assistance (Claude Code), including
the fix, the regression test, and this PR description. All commands
listed above were run and their output verified directly.

ExtensionPrimitive.install_from_directory/install_from_zip in the
bundler never called ExtensionManager.scaffold_config, so an
extension's provides.config templates were deployed only when
installed with `specify extension add`, not via `specify bundle
install`. Call scaffold_config after a successful install in both
code paths, matching the extension-add command flow.

Fixes github#4283

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.

Pull request overview

Fixes bundle-installed extensions failing to scaffold declared configuration templates.

Changes:

  • Scaffolds config after directory and ZIP installs.
  • Adds bundled-directory regression coverage.
  • Updates mocks to return manifest IDs.
Show a summary per file
File Description
src/specify_cli/bundler/services/primitives.py Scaffolds extension config after installation.
tests/unit/test_bundler_primitives.py Adds regression coverage and updates mocks.

Review details

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Balanced

project = tmp_path / "project"
ext_source = tmp_path / "ext-source"
_write_extension_with_config(ext_source)
monkeypatch.setattr(assets, "_locate_bundled_extension", lambda cid: ext_source)
@KSchlobohm

Copy link
Copy Markdown
Contributor

Please add a regression test for the catalog/ZIP install path. The current test covers only an extension already bundled with Spec Kit, while the reported reproduction uses an extension downloaded from the catalog. The new test should confirm that installing the downloaded ZIP creates the extension's configuration file. Once updated, we'll take another look.

Adds a regression test that exercises the catalog-download branch
(install_from_zip) rather than the bundled-directory branch, per
review feedback that the reported reproduction used a catalog-resolved
extension.
@chelsealong

Copy link
Copy Markdown
Contributor Author

Added test_catalog_extension_install_scaffolds_config in tests/unit/test_bundler_primitives.py, which forces the catalog/ZIP branch (_locate_bundled_extension returns None, ExtensionCatalog.download_extension supplies a real ZIP built from an extension with a provides.config entry) and asserts the scaffolded config file lands on disk. Confirmed it fails without the fix and passes with it; full suite (7310 passed) and ruff on touched files are unchanged from before (same 4 pre-existing findings noted in the PR description).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: bundle install does not scaffold extension config templates

4 participants