Skip to content

feat(presets): support regex resource selectors - #4776

Open
lmtyy wants to merge 8 commits into
github:mainfrom
lmtyy:feat/4659-regex-preset-selectors
Open

lmtyy wants to merge 8 commits into
github:mainfrom
lmtyy:feat/4659-regex-preset-selectors

Conversation

@lmtyy

@lmtyy lmtyy commented Sep 28, 2026

Copy link
Copy Markdown

Add regex: selectors for preset templates, scripts, and commands. Existing exact-name matching remains unchanged; regex patterns are validated and matched against complete resource names.

Command selectors are expanded to concrete lower-layer command names before registration, keeping selector expressions out of command filenames and registry records. Reconciliation and diagnostics handle matched resources across preset lifecycle changes.

Closes #4659

Testing

  • Tested locally with uv run specify --help
  • Ran the full test suite: 8415 passed, 211 skipped
  • Ran targeted selector, resolver, manifest, and command lifecycle tests: 376 passed
  • Ran Python parity tests: 98 passed, 2 skipped
  • Ran Ruff checks, ShellCheck, PowerShell syntax parsing, Python compilation, and git diff --check

AI Disclosure

  • I did not use AI assistance for this contribution
  • I did use AI assistance (fill in the disclosure below)

AI disclosure: Implemented with Hermes Agent (Nous Research), using the gpt-6-Astra model in an interactive, human-supervised workflow. AI assistance was used for code changes, tests, debugging, and validation; the contributor should review the final diff before merging.

Add regex:<pattern> selectors for preset templates, scripts, and commands while preserving exact-name behavior. Validate regex patterns and use full-name matching. Expand command selectors to concrete lower-layer commands before registration and reconcile affected commands across preset lifecycle changes.

Add selector, resolver, command lifecycle, and diagnostic coverage. Verified with the full test suite (8415 passed, 211 skipped), Ruff, ShellCheck, PowerShell syntax parsing, Python compilation, and git diff --check.

Closes github#4659

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.

Copilot review overview

🟡 Changes recommended

Command resolution, skill registration, diagnostics, warnings, and lifecycle reconciliation have unresolved functional gaps.

Review effort: Balanced
Findings: 3 High severity · 2 Medium severity

Open (5)
What changed in this PR

Adds regex: resource selectors to preset resolution and command materialization.

Changes:

  • Validates and full-matches regex selectors.
  • Expands command selectors and adds lifecycle reconciliation.
  • Adds resolver and selector coverage.
File Description
tests/​specify_cli/​presets/​test_regex_selectors.py Tests selector matching and resolution.
src/​specify_cli/​presets/​command_set_priority.py Reconciles commands after priority changes.
src/​specify_cli/​presets/​command_info.py Displays selector matches.
src/​specify_cli/​presets/​command_enable.py Reconciles enabled preset commands.
src/​specify_cli/​presets/​_selectors.py Implements selector helpers.
src/​specify_cli/​presets/​_resolver.py Resolves regex template and script layers.
src/​specify_cli/​presets/​_manifest.py Validates regex expressions.
src/​specify_cli/​presets/​_manager.py Integrates expansion into lifecycle handling.
src/​specify_cli/​presets/​_manager_commands.py Expands command selectors.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/specify_cli/presets/_resolver.py Outdated
Comment thread src/specify_cli/presets/_resolver.py Outdated
Comment thread src/specify_cli/presets/command_info.py Outdated
Comment thread src/specify_cli/presets/_manager.py
Comment thread src/specify_cli/presets/_manager_commands.py
@mnriem

mnriem commented Sep 28, 2026

Copy link
Copy Markdown
Collaborator

Please address Copilot feedback

Add regex:<pattern> selectors for preset templates, scripts, and commands while preserving exact-name behavior. Resolve all matching declarations in manifest order, expand command selectors before registration, and reconcile matches after preset and extension changes. Add selector diagnostics and regression coverage.

Closes github#4659
@lmtyy

lmtyy commented Sep 28, 2026

Copy link
Copy Markdown
Author

Addressed all five Copilot review findings:

  • Added command regex selectors to normal layer resolution and composition.
  • Preserved all matching regex declarations in manifest order.
  • Fixed the invalid _selectors imports used by diagnostics.
  • Reused selector expansion for AI-skill registration.
  • Added selector-aware reconciliation for relevant preset/extension lifecycle changes.

Also added regression coverage for overlapping selectors, command composition strategies, diagnostics, AI-skill registration, and lifecycle state changes.

Validation:

  • 8624 passed, 17 skipped
  • 183 targeted regression tests passed
  • Ruff passed
  • Python compilation passed
  • git diff --check passed

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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@mnriem mnriem added the triage-can-wait Verdict: valid and in-scope but deprioritized; held behind the evidence gate label Sep 28, 2026
@lmtyy

lmtyy commented Sep 29, 2026

Copy link
Copy Markdown
Author

All previous Copilot review findings have been addressed and the updated test suite is passing. The latest Copilot re-review appears to have failed due to a review error, so the PR is ready for another review when convenient.

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.

Copilot review overview

🟡 Changes recommended

Selector lifecycle paths can leave stale commands or skills and several acceptance criteria remain incomplete.

Review effort: Balanced
Findings: 6 Medium severity

Open (6)

Comment thread src/specify_cli/presets/_manager.py Outdated
Comment thread src/specify_cli/presets/_manager.py
Comment thread src/specify_cli/presets/_manager_skills.py
Comment thread src/specify_cli/presets/command_info.py Outdated
Comment thread src/specify_cli/presets/command_set_priority.py Outdated
Comment thread tests/specify_cli/presets/test_regex_selector_lifecycle.py Outdated
@mnriem

mnriem commented Sep 29, 2026

Copy link
Copy Markdown
Collaborator

Please address Copilot feedback

Add install-time warnings for unmatched template, script, and command selectors while keeping zero-match selectors non-fatal.

Reconcile constitution snapshots when regex selectors target the constitution template, and preserve concrete command matches during AI skill registration and reconciliation.

Include extension manifest-declared resources in selector diagnostics so reported matches stay consistent with actual resolver behavior.

Handle preset and extension lifecycle changes using selector-aware reconciliation to remove stale command and skill artifacts and restore newly matching resources.

Make priority-change reconciliation failures consistent with existing lifecycle behavior and add regression coverage for enable, disable, priority, diagnostics, constitution, and skill scenarios.

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.

Copilot review overview

🟡 Changes recommended

Selector lifecycle transitions can leave stale or untracked command and skill artifacts.

Review effort: Balanced
Findings: 3 Medium severity · 2 Low severity

Open (5)
Resolved since last review (6)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Stale artifacts remain when selectors lose all matches

src/​specify_cli/​extensions/​_commands.py:121

This refresh runs only after the extension state has changed, so a selector whose sole lower layer was disabled or removed now expands to nothing. register_enabled_presets_for_agent() never sees the previously materialized concrete name, and its reconciler also skips names with no remaining layers, leaving the old command/SKILL artifact active. Capture the pre-mutation matches (or persisted registrations), reconcile the union with post-mutation matches, and explicitly unregister names that no longer resolve.

Comment thread src/specify_cli/presets/_manager_commands.py
Comment thread src/specify_cli/presets/_manager_commands.py Outdated
Comment thread src/specify_cli/presets/_manager_commands.py Outdated
Comment thread src/specify_cli/presets/command_info.py
Comment thread tests/specify_cli/presets/test_regex_selector_lifecycle.py
text
- Expand regex command selectors before resolving layers and registering AI skills.
- Track concrete command and skill names for regex-owned and composed commands.
- Reconcile generated artifacts when presets are disabled or re-enabled.
- Preserve the Templates count in preset info output.
- Add end-to-end tests for selector artifact lifecycle and composition.

Validation:
- 8,626 passed, 17 skipped
- Ruff checks passed
- git diff --check passed

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.

Copilot review overview

🟡 Changes recommended

Selector cleanup, disabled-extension resolution, and installation rollback still have correctness gaps.

Review effort: Balanced
Findings: 1 High severity · 3 Medium severity

Open (4)
Resolved since last review (5)
Previously missed (2)

In code that hasn't changed since last review

Medium severity Selector expansion failure leaves partial installation

src/​specify_cli/​presets/​_manager.py:456

Selector diagnostics and expansion run after the preset directory and registry entry are created but before the existing rollback guard. _expand_command_selectors() deliberately raises when artifact enumeration fails, so such a failure leaves an enabled, partially installed preset behind. Move both calls into the guarded installation phase so the established cleanup executes.

Medium severity Re-enumeration causes false failure after registration

src/​specify_cli/​presets/​_manager.py:507

This repeats artifact enumeration outside the transactional registration block. If inventory changes or becomes unreadable after registration, the command reports installation failure even though the preset and artifacts are already committed. Reuse the concrete command_templates computed for registration.

Comment thread src/specify_cli/presets/_resolver.py Outdated
Comment thread src/specify_cli/integrations/_command_upgrade_layout.py
Comment thread src/specify_cli/presets/_manager_commands.py
Comment on lines +86 to +89
manager.registry.update(preset_id, {"enabled": False})
if names:
manager._reconcile_composed_commands(sorted(names))
manager._reconcile_skills(sorted(names))
@mnriem

mnriem commented Sep 29, 2026

Copy link
Copy Markdown
Collaborator

Please address Copilot feedback

- Track expanded selector matches through preset and extension lifecycle operations.
- Clean up generated command and skill artifacts when providers are disabled or removed.
- Preserve registry provenance and restore prior installs when preset or extension installation fails.
- Add lifecycle and failure-injection regression tests.
@lmtyy

lmtyy commented Sep 30, 2026

Copy link
Copy Markdown
Author

@mnriem I've addressed the latest Copilot feedback in commit 1ce59c1, including the disabled-extension fallback, include_disabled handling, stale selector tracking/cleanup, and install rollback coverage.

When convenient, could you please re-request the Copilot review? Thanks!

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.

Copilot review overview

🟡 Changes recommended

Selector rollback, disabled-state handling, and extension-layer deduplication contain unresolved correctness issues.

Review effort: Balanced
Findings: 1 High severity · 5 Medium severity

Open (6)
Resolved since last review (3)

Comment thread src/specify_cli/extensions/__init__.py
Comment on lines +150 to +155
if not meta.get("enabled", True):
# Disabled presets are only relevant to callers that explicitly
# request installed-but-disabled provenance.
if include_disabled:
affected.append(preset_id)
continue
Comment on lines +487 to +497
old_manifest = PresetManifest(dest_dir / "preset.yml")
old_commands = sorted(
item["name"]
for item in old_manifest.templates
if item.get("type") == "command"
and isinstance(item.get("name"), str)
and not is_regex_selector(item["name"])
)
if old_commands:
self._reconcile_composed_commands(old_commands)
self._reconcile_skills(old_commands)
and extension_metadata.get("enabled", True)
):
candidate = self._find_unregistered_extension_command(template_name)
if candidate is not None:
Comment on lines +99 to +103
console.print(
f"[yellow]Preset '{preset_id}' disabled; artifact cleanup failed. "
"Tracked files remain recorded for retry.[/yellow]"
)
return

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.

Comment thread src/specify_cli/extensions/__init__.py
Comment on lines +809 to +810
if candidate is None and ext_meta is None:
candidate = self._find_unregistered_extension_command(template_name)
Comment on lines +71 to +80
suffix = ".sh" if resource_type == "script" else ".md"
core_roots = [
resolver.templates_dir / ("scripts" if resource_type == "script" else ""),
]
for root in core_roots:
for path in root.glob("**/*") if root.is_dir() else []:
if path.is_file() and path.name.endswith(suffix):
candidates.add(
os.path.relpath(path, root)[: -len(suffix)].replace(os.sep, "-")
)
Comment on lines +64 to 66
affected_commands.update(
manager._collect_selector_command_names(PresetResolver(project_root))
)

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.

Comment on lines +119 to +121
agent = load_init_options(project_root).get("ai")
if agent:
PresetManager(project_root).register_enabled_presets_for_agent(agent)
Comment on lines 967 to 971
if (
tmpl.get("type") == "template"
and tmpl.get("name") == "constitution-template"
):
removed_constitution = True
Comment on lines +838 to +839
candidate = self._find_unregistered_extension_command(template_name)
if candidate is not None:
@mnriem

mnriem commented Sep 30, 2026

Copy link
Copy Markdown
Collaborator

Please address Copilot feedback

This branch has not been deployed

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

Labels

triage-can-wait Verdict: valid and in-scope but deprioritized; held behind the evidence gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support regex selectors for preset commands, templates, and scripts

3 participants