Skip to content

feat(extensions): add --force flag to extension add for overwrite reinstall#2530

Open
grissiom wants to merge 5 commits into
github:mainfrom
grissiom:feat/extension-add-force
Open

feat(extensions): add --force flag to extension add for overwrite reinstall#2530
grissiom wants to merge 5 commits into
github:mainfrom
grissiom:feat/extension-add-force

Conversation

@grissiom
Copy link
Copy Markdown

feat(extensions): add --force flag to extension add for overwrite reinstall

Add --force support to specify extension add that allows overwriting an already-installed extension without manually removing it first.

  • install_from_directory() and install_from_zip() accept force=True, automatically calling remove() before installation
  • The --force CLI flag works with all install modes (--dev, --from URL, bundled, and catalog)
  • Config files (*-config.yml) are preserved across force reinstall
  • Error message suggests --force when extension is already installed
  • 6 new tests covering unit and CLI force reinstall flows

Description

If the extension is updated, --force could eliminate the remove/install loop and simplify the workflow.

Testing

  • Tested locally with uv run specify --help
  • Ran existing tests with uv sync && uv run pytest
  • Tested with a sample project (if applicable)

AI Disclosure

  • I did not use AI assistance for this contribution
  • I did use AI assistance (describe below)

Generated by Copilot(model: DeepSeek-V4).

@grissiom grissiom requested a review from mnriem as a code owner May 12, 2026 15:26
@mnriem mnriem requested a review from Copilot May 12, 2026 16:27
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds --force support to specify extension add so users can overwrite/reinstall an already-installed extension in one step, while preserving *-config.yml files across the reinstall.

Changes:

  • Add force parameter to ExtensionManager.install_from_directory() / install_from_zip() and plumb --force through the CLI.
  • Preserve extension config files across force reinstalls via .specify/extensions/.backup/<ext_id> restore logic.
  • Add unit + CLI tests for force reinstall behavior and updated duplicate-install error messaging.
Show a summary per file
File Description
src/specify_cli/extensions.py Implements force reinstall path, updated duplicate install error text, and config backup restore.
src/specify_cli/__init__.py Adds --force flag to specify extension add and passes it through to install routines.
tests/test_extensions.py Adds tests for force reinstall flows (directory/zip) and CLI coverage for extension add --dev --force.

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

Comment thread src/specify_cli/extensions.py Outdated
Comment thread src/specify_cli/extensions.py
Comment thread tests/test_extensions.py Outdated
@mnriem
Copy link
Copy Markdown
Collaborator

mnriem commented May 12, 2026

Please address Copilot feedback

@grissiom
Copy link
Copy Markdown
Author

@mnriem fixed the 3 comments, please review it again. Thanks~

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot's findings

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

Comment thread src/specify_cli/extensions.py Outdated
Comment on lines +1208 to +1212
# Restore config files from backup when reinstalling with --force
if force:
backup_config_dir = self.extensions_dir / ".backup" / manifest.id
if backup_config_dir.exists():
for cfg_file in backup_config_dir.iterdir():
Copy link
Copy Markdown
Collaborator

@mnriem mnriem left a comment

Choose a reason for hiding this comment

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

Please address Copilot feedback

@grissiom
Copy link
Copy Markdown
Author

@mnriem Thank you for your review. The bug is fixed. Please review it again. Thanks~

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot's findings

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

# validation failure doesn't leave the user with a half-uninstalled
# extension (configs stranded in .backup/).
did_remove = False
if force and self.registry.is_installed(manifest.id):
Comment thread src/specify_cli/extensions.py Outdated
backup_config_dir = self.extensions_dir / ".backup" / manifest.id
if backup_config_dir.exists():
for cfg_file in backup_config_dir.iterdir():
if cfg_file.is_file():
Comment thread src/specify_cli/__init__.py Outdated
speckit_version = get_speckit_version()

if force:
console.print("[yellow]--force:[/yellow] Will overwrite if already installed\n")
Copy link
Copy Markdown
Collaborator

@mnriem mnriem left a comment

Choose a reason for hiding this comment

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

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

@grissiom grissiom force-pushed the feat/extension-add-force branch from 355a649 to 1ed5439 Compare May 23, 2026 04:33
@grissiom
Copy link
Copy Markdown
Author

Nice findings @mnriem , all issues fixed and please review it again. Thanks~

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot's findings

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

Comment thread src/specify_cli/extensions.py Outdated
Comment on lines 1185 to 1191
if backup_config_dir.exists():
shutil.rmtree(backup_config_dir)
did_remove = self.remove(manifest.id)

# Install extension
dest_dir = self.extensions_dir / manifest.id
if dest_dir.exists():
Comment on lines +1220 to +1226
for cfg_file in backup_config_dir.iterdir():
if cfg_file.is_file() and (
cfg_file.name.endswith("-config.yml") or
cfg_file.name.endswith("-config.local.yml")
):
shutil.copy2(cfg_file, dest_dir / cfg_file.name)
shutil.rmtree(backup_config_dir)
Comment thread tests/test_extensions.py
# Install once
manifest1 = manager.install_from_directory(
extension_dir, "0.1.0", register_commands=False
)
@mnriem
Copy link
Copy Markdown
Collaborator

mnriem commented May 27, 2026

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

grissiom added 5 commits May 30, 2026 12:17
…nstall

Add --force support to `specify extension add` that allows overwriting
an already-installed extension without manually removing it first.

- install_from_directory() and install_from_zip() accept force=True,
  automatically calling remove() before installation
- The --force CLI flag works with all install modes (--dev, --from URL,
  bundled, and catalog)
- Config files (*-config.yml) are preserved across force reinstall
- Error message suggests --force when extension is already installed
- 6 new tests covering unit and CLI force reinstall flows
- Remove unused `backup_config_dir` variable assignment (Ruff F841)
- Defer `remove()` until after `_validate_install_conflicts()` to prevent
  data loss if validation fails mid-reinstall
- Use `TemporaryDirectory` instead of `NamedTemporaryFile` in ZIP test
  to avoid Windows file-locking failures
When --force is used but the extension is not already installed, the
backup restore/cleanup should not run. Previously it could resurrect
stale config files from a previous removal and delete the backup
directory unnecessarily.
- Clear stale backup dir before remove() so only fresh backups are restored
- Restore only config files (*-config.yml, *-config.local.yml) from backup
- Remove trailing \n from --force console message (console.print adds newline)
- Use is_dir() before rmtree/iterdir on backup path to avoid crashes
  when .backup/<id> exists as a file or symlink
- Remove unused manifest1 variable in test_install_force_reinstall
@grissiom grissiom force-pushed the feat/extension-add-force branch from 93cf642 to bd1cea9 Compare May 30, 2026 04:19
@grissiom
Copy link
Copy Markdown
Author

Thank you @mnriem , all feedback fixed and the PR branch rebased to the main. Please review it again. Thanks~

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.

3 participants