feat(extensions): scaffold config templates on extension add/enable

[mvanhorn]

Description

Rework of #1929 per @mnriem's feedback. Instead of a standalone specify extension init command, config scaffolding now runs automatically as part of the extension lifecycle:

• extension add: after installing an extension, config templates from provides.config are deployed to .specify/
• extension enable: when re-enabling a disabled extension, missing config templates are deployed

Existing user-customized config files are never overwritten. Extensions without a provides.config section are unaffected.

Changes:

• ExtensionManifest.config property reads provides.config from the manifest
• ExtensionManager.scaffold_config() copies config templates to the project, skipping existing files
• extension_add replaces the "Configuration may be required" warning with automatic deployment
• extension_enable deploys config on re-enable

Video Demo

The demo shows: config template exists in extension dir, no project config yet, scaffold_config deploys it, re-running preserves existing config (idempotent).

Testing

• Tested locally with uv run specify --help
• Ran existing tests with uv sync --extra test && uv run python -m pytest - all 890 tests pass
• Added 4 new tests covering scaffold_config: deploy, preserve existing, no config section, missing template
• Tested with a sample project (see demo above)

AI Disclosure

• I did use AI assistance (describe below)

This contribution was developed with AI assistance (Claude Code + Codex CLI).

Closes #1929

[mnriem]

On a side note you moved us into the two thousands! ;)

[Copilot]

Pull request overview

This PR updates the extension lifecycle so that configuration templates declared by an extension (provides.config) are automatically scaffolded into a project’s .specify/ directory during specify extension add and specify extension enable, without overwriting existing user config.

Changes:

• Add ExtensionManifest.config to expose provides.config from extension.yml.
• Add ExtensionManager.scaffold_config() to copy declared config templates into .specify/, skipping existing files.
• Invoke config scaffolding from extension add and extension enable, and add tests for scaffold behavior.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 5 comments.

File	Description
src/specify_cli/extensions.py	Adds manifest accessor for config and implements scaffold_config() to deploy templates.
src/specify_cli/__init__.py	Wires scaffolding into extension add and extension enable CLI flows and prints status output.
tests/test_extensions.py	Adds new tests covering config scaffolding deploy/preserve/no-config/missing-template cases.
docs/screenshots/scaffold-config-demo.gif	Adds demo asset showing scaffolding behavior.

Comments suppressed due to low confidence (1)

src/specify_cli/extensions.py:1158

• scaffold_config() assumes every config_entry is a dict (config_entry.get(...)). Since provides.config isn’t currently validated, a non-dict entry will raise AttributeError here. Even with validation, it’s safer to defensively skip non-dict entries (or raise ValidationError) to keep lifecycle commands from crashing on bad extensions.

        for config_entry in manifest.config:
            template_name = config_entry.get("template", "")
            target_name = config_entry.get("name", template_name)
            if not template_name:
                continue

---

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

[mvanhorn]

Ha, PR #2000! Didn't even notice. Lucky number.

[mnriem]

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

[mvanhorn]

Addressed all Copilot feedback in 86606ef:

1. Path traversal protection - scaffold_config() now rejects template/target names with .. segments or absolute paths
2. File type check - uses is_file() instead of exists() to reject directory templates
3. Manifest validation - config property normalizes malformed entries (non-list or non-dict values return [])
4. Richer messaging - returns (deployed, skipped) tuple so the CLI distinguishes "already exists" from "template missing"
5. Security tests - added test coverage for path traversal rejection, directory template rejection, and malformed manifest handling

[Copilot]

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (2)

src/specify_cli/extensions.py:1164

• template_name/target_name are assumed to be strings, but manifest data is untrusted input. If either value is non-string (e.g., int/None), Path(template_name) will raise TypeError and abort scaffolding. Add type checks (or coerce to str only when appropriate) and skip invalid entries to keep scaffolding robust.

        for config_entry in manifest.config:
            template_name = config_entry.get("template", "")
            target_name = config_entry.get("name", template_name)
            if not template_name:
                continue

src/specify_cli/extensions.py:1174

• The current path traversal protection relies on Path(...).is_absolute() and checking for '..' segments. This misses some edge cases (e.g., Windows drive-relative paths like C:foo, and symlinks inside the extension that resolve outside ext_dir). Prefer the same resolve() + relative_to() containment pattern already used for ZIP extraction in this file, and consider rejecting symlinks for templates before copying.

            # Reject path traversal and absolute paths
            if Path(template_name).is_absolute() or ".." in Path(template_name).parts:
                continue
            if Path(target_name).is_absolute() or ".." in Path(target_name).parts:
                continue

            template_path = ext_dir / template_name
            if not template_path.exists() or not template_path.is_file():
                continue

---

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

[mvanhorn]

Rebased and addressed the review. Since #3014 split extensions.py into the extensions/ package while this sat, this is a port rather than a rebase: the manager and manifest changes now land in extensions/__init__.py and the CLI wiring in extensions/_commands.py. Same behaviour, new home.

On the eight comments:

• Consistent return -- scaffold_config() returns (deployed, skipped_existing, failed) on every path, including the missing-manifest case, which previously returned a bare [] and would have raised on unpack.
• Misleading message -- the new failed list separates "already exists (preserved)" from "template missing/invalid", and both extension_add and extension_enable report them distinctly.
• Path containment -- template paths must resolve inside the extension dir and targets inside .specify/, checked with resolve() + relative_to() rather than a .. substring test.
• File type -- symlinks and non-regular files are rejected before any copy.
• Manifest shape -- ExtensionManifest.config returns [] unless provides.config is a list of dicts, and scaffold_config() surfaces a malformed section as a failure instead of iterating it.
• enable crash -- extension_enable catches manifest errors and warns instead of tracebacking.
• Tests -- the malformed-manifest test now uses a valid manifest with a non-list provides.config, and there are new cases for symlink templates, path traversal, absolute paths, directory templates, and the missing-manifest tuple shape.

pytest is 5282 passed / 172 skipped on the full suite.

[Copilot]

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 5
• Review effort level: Medium

[mnriem]

Please address Copilot feedback

[mvanhorn]

1db0301 addresses these, and rebases onto main since the branch had drifted 75 commits.

The config location was the real one. ConfigManager._get_project_config() reads .specify/extensions/<id>/<id>-config.yml, so writing to the .specify/ root put the file somewhere the extension never looks. Deployment moved there, and two existing tests that asserted the old path are updated rather than worked around.

Containment is now checked component by component before .specify becomes the root, matching the shared_infra safe-write path. Resolving it first and trusting the result meant a symlinked component let every target pass relative_to and copy2 write outside the project.

mkdir moved inside the OSError handler, so a nested target whose parent cannot be created is recorded as failed instead of raising after extension add already installed the extension.

The "Configuration may be required" warning is conditional now. It ran unconditionally right after the scaffolding output, so it contradicted a successful scaffold and fired for extensions with no provides.config at all.

New tests cover the corrected location, a symlinked config root, and an uncreatable nested target. Suite is 6019 passing.

The earlier comments on src/specify_cli/extensions.py predate the split into the package; the return-type, path-traversal, symlink-template and manifest-shape points they raised are all handled in the current extensions/__init__.py.

[Copilot]

Review details

Comments suppressed due to low confidence (6)

src/specify_cli/extensions/_commands.py:734

• This condition still emits the generic configuration warning for extensions with no provides.config, because all three result lists are empty. That contradicts the comment immediately above and makes unaffected extensions appear to need setup. Restrict this warning to actual scaffolding failures.

        if failed or not (deployed or skipped):

src/specify_cli/extensions/init.py:2560

• name is required by the manifest API, but defaulting it to template makes a missing name look successful: the template already exists at that same path, so the entry is reported as “skipped/preserved” and no config is deployed. Treat a missing name as malformed instead.

            target_name = config_entry.get("name", template_name)

src/specify_cli/extensions/_commands.py:715

• The new tests call scaffold_config() directly, so the main extension add lifecycle integration and its CLI behavior are not exercised. Add a CLI test that installs an extension with config and verifies deployment, plus a no-config case so warning regressions in this branch are caught.

        # Scaffold config templates automatically
        deployed, skipped, failed = manager.scaffold_config(manifest.id)

src/specify_cli/extensions/_commands.py:2244

• No added test re-enables a disabled extension through the CLI; all scaffolding tests invoke the manager directly. Add an enable-path test that removes a generated config, disables/re-enables the extension, and verifies the missing config is restored without altering existing config.

    # Scaffold config templates on enable
    try:
        deployed, skipped, failed = manager.scaffold_config(extension_id)

tests/test_extensions.py:10473

• This unconditionally creates a symlink, but the suite runs on Windows and already uses can_create_symlink() to skip when the process lacks symlink privileges (for example, tests/test_extensions.py:1437-1443 and 3573-3574). Guard this test the same way or it can fail before exercising scaffolding.

        (ext_dir / "config-link.yml").symlink_to(real_template)

tests/test_extensions.py:10527

• Directory symlink creation is also unconditional here. The test matrix includes Windows, while this file’s established pattern is to call can_create_symlink() and skip when privileges are unavailable (tests/test_extensions.py:52-63, 1437-1443). Add that guard before creating .specify.

        (project / ".specify").symlink_to(outside, target_is_directory=True)

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Medium

[mnriem]

Please address Copilot feedback