Skip to content

Resolve extension hooks with native pre/post scripts - #4883

Open
mnriem wants to merge 26 commits into
github:mainfrom
mnriem:mnriem-scripted-extension-hooks
Open

mnriem wants to merge 26 commits into
github:mainfrom
mnriem:mnriem-scripted-extension-hooks

Conversation

@mnriem

@mnriem mnriem commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator

Description

Replace repeated in-prompt parsing and ordering of extension pre/post hooks with native, self-contained Bash, PowerShell, and Python dispatch scripts. Agent command templates consume the ordered JSON result and invoke the selected extension commands; the scripts do not attempt to execute agent commands. Render the script path across command, skill, and preset installations, and serialize the supported canonical configuration for the native resolvers. Invalid or unsupported configuration fails explicitly rather than silently dropping hooks.

The ten core hook-bearing command templates shrink from 18,736 to 14,680 words (4,056 fewer; 21.6%). Tests cover priority/order, disabled/conditional/optional hooks, invalid input, cross-runtime parity, integration rendering, and package contents; the GitHub extension command stays in parity with its core counterpart.

Testing

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

Equivalent local checks: .venv/bin/specify --help — passed; LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest tests -q --tb=short — 9,839 passed, 19 skipped, 62 warnings; git diff --check — passed. The exact checklist commands and an agent-driven sample-project slash-command run were not performed, so their boxes remain unchecked.

Manual test results

Agent: Not run | OS/Shell: macOS / Bash, PowerShell and Python resolver behavior covered by automated tests

Command tested Notes
Agent-driven slash commands Not run; automated command-rendering and hook-dispatch tests passed.

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 GitHub Copilot using GPT-6 Sol in autonomous agent mode (reasoning effort not explicitly configured; default settings). AI authored the implementation, tests, and documentation; no human line-by-line review is claimed.

Replace repeated command-template hook resolution with ordered native Bash, PowerShell, and Python dispatchers. Keep configuration canonical, render dispatch paths across agent integrations, and exercise parity and rejection cases.

Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 8, 2026 13:41

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.

🟡 Changes recommended

Extension-rendered paths target nonexistent scripts, and native resolver parsing still has cross-runtime correctness gaps.

3 open findings
What changed in this PR

Replaces prompt-based extension-hook parsing with native Bash, PowerShell, and Python dispatchers that return ordered hook metadata as JSON.

Changes:

  • Adds cross-runtime pre/post hook resolvers and canonical configuration serialization.
  • Renders dispatcher paths across commands, skills, presets, and extensions.
  • Updates documentation and integration/parity coverage.
File Description
scripts/​bash/​pre-hooks.sh Adds Bash hook resolver.
scripts/​bash/​post-hooks.sh Adds Bash post-hook wrapper.
scripts/​powershell/​pre-hooks.ps1 Adds PowerShell hook resolver.
scripts/​powershell/​post-hooks.ps1 Adds PowerShell post-hook wrapper.
scripts/​python/​pre_hooks.py Adds Python hook resolver.
scripts/​python/​post_hooks.py Adds Python post-hook wrapper.
src/​specify_cli/​integrations/​base.py Renders hook-script placeholders.
src/​specify_cli/​agents.py Resolves placeholders in skills and wrapped commands.
src/​specify_cli/​extensions/​__init__.py Emits canonical, unwrapped hook YAML.
templates/​commands/​analyze.md Delegates hooks to dispatch scripts.
templates/​commands/​checklist.md Delegates hooks to dispatch scripts.
templates/​commands/​clarify.md Delegates hooks to dispatch scripts.
templates/​commands/​constitution.md Delegates hooks to dispatch scripts.
templates/​commands/​converge.md Delegates hooks to dispatch scripts.
templates/​commands/​implement.md Delegates hooks to dispatch scripts.
templates/​commands/​plan.md Delegates hooks to dispatch scripts.
templates/​commands/​specify.md Delegates hooks to dispatch scripts.
templates/​commands/​tasks.md Delegates hooks to dispatch scripts.
templates/​commands/​taskstoissues.md Delegates hooks to dispatch scripts.
extensions/​github/​commands/​speckit.github.taskstoissues.md Keeps extension command hook parity.
extensions/​EXTENSION-API-REFERENCE.md Documents dispatcher JSON contract.
docs/​reference/​extensions.md Documents resolver behavior and YAML subset.
design/​integration.md Defines hook-script rendering architecture.
tests/​test_hook_dispatch_scripts.py Covers resolver behavior and parity.
tests/​test_command_template_hooks.py Covers template rendering and dispatch instructions.
tests/​extensions/​github/​test_github_extension.py Updates GitHub extension contract tests.
tests/​integrations/​test_integration_agy.py Verifies generated dispatcher references.
tests/​integrations/​test_integration_alquimia.py Verifies generated dispatcher references.
tests/​integrations/​test_integration_base_markdown.py Updates installed-file inventory.
tests/​integrations/​test_integration_base_skills.py Covers skill rendering and inventory.
tests/​integrations/​test_integration_base_toml.py Updates installed-file inventory.
tests/​integrations/​test_integration_base_yaml.py Updates installed-file inventory.
tests/​integrations/​test_integration_claude.py Verifies generated dispatcher references.
tests/​integrations/​test_integration_cline.py Updates installed-file inventory.
tests/​integrations/​test_integration_codex.py Verifies generated dispatcher references.
tests/​integrations/​test_integration_copilot.py Covers dispatch rendering and inventory.
tests/​integrations/​test_integration_generic.py Covers dispatch rendering and inventory.
tests/​integrations/​test_integration_hermes.py Verifies generated dispatcher references.
tests/​integrations/​test_integration_junie.py Updates installed-file inventory.

🧠 Review effort: Balanced


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

Comment thread src/specify_cli/integrations/base.py Outdated
Comment thread scripts/bash/pre-hooks.sh Outdated
Comment thread scripts/powershell/pre-hooks.ps1 Outdated
Keep shared dispatch paths rooted when extension commands render, decode Bash backslash escapes, and reject implicitly typed identifiers in native resolvers. Add cross-runtime and extension rendering regressions.

Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 8, 2026 15:58
@mnriem

mnriem commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed the three findings in c240663: extension-rendered pre/post dispatch paths now stay rooted at the shared .specify/scripts/ location; Bash decodes double-quoted YAML backslash escapes; and native Bash/PowerShell resolvers reject implicitly typed non-string extension/command identifiers. Added before-and-after regression coverage for extension command/skill rendering, escaped backslashes, typed values, and quoted string identifiers. Validation: LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest tests -q --tb=short — 9,887 passed, 19 skipped, 62 warnings; bash -n scripts/bash/pre-hooks.sh scripts/bash/post-hooks.sh and git diff --check passed. Posted on behalf of @mnriem by GitHub Copilot (GPT-6 Sol, autonomous agent mode, default reasoning settings); AI authored the fix, regression tests, and this review-round summary, without a human line-by-line review claim.

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.

🟡 Changes recommended

The Bash resolver breaks on default macOS Bash, and configuration migration and cross-runtime condition semantics remain inconsistent.

1 open finding
3 resolved since last review
Previously missed (2)

In code that hasn't changed since last review

Medium severity Validate condition types to keep hook behavior consistent

scripts/​python/​pre_hooks.py:49

condition is never type-validated. For example, YAML condition: false is loaded here as a falsey boolean and the hook executes, while the Bash/PowerShell parsers retain false as non-empty text and skip the same hook. Since the documented field is a string or null, reject non-string/non-null values before filtering so the selected script variant cannot change whether a mandatory hook runs.

Medium severity Existing configs are incompatible after parser migration

src/​specify_cli/​extensions/​__init__.py:5729

This only canonicalizes .specify/extensions.yml on its next write. Projects upgraded from an earlier release can still have valid PyYAML output wrapped at the previous default width; both new native parsers reject those continuation lines, so upgrading the integration can make every hook check fail until some unrelated extension command rewrites the file. Migrate/rewrite existing configuration during the shared-infrastructure upgrade, or keep the native parsers compatible with the previously emitted layout.

🧠 Review effort: Balanced


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

Comment thread scripts/bash/pre-hooks.sh Outdated
Use Bash 3-compatible scalar checks, validate condition types across runtimes, and canonicalize existing extension configuration when shared infrastructure is installed or upgraded. Guard invalid and symlinked configurations and cover old wrapped YAML with executable resolver regressions.

Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 8, 2026 16:53
@mnriem

mnriem commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed this review in 2865187. The Bash resolver no longer uses Bash 4-only lowercase expansion; a regression invokes both hook phases through macOS /bin/bash 3.2. All three resolvers now reject non-string/non-null condition values consistently. Shared-infrastructure installation/upgrade canonicalizes existing valid extension YAML, including legacy wrapped prompts, before native dispatch; invalid and symlinked config is not overwritten. Validation: LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest tests -q --tb=short — 9,928 passed, 19 skipped, 62 warnings before the final symlink guard; after that guard, the affected hook, upgrade, and template suites passed (300 passed). /bin/bash -n scripts/bash/pre-hooks.sh scripts/bash/post-hooks.sh and git diff --check passed. Posted on behalf of @mnriem by GitHub Copilot (GPT-6 Sol, autonomous agent mode, default reasoning settings); AI authored this fix, regression tests, documentation update, and review-round summary, with no human line-by-line review claimed.

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.

🟡 Changes recommended

Configuration migration can write through a symlinked parent, and resolver validation and priority ordering are not consistent across runtimes.

1 open finding
1 resolved since last review
Previously missed (3)

In code that hasn't changed since last review

Medium severity Validate YAML fields in every event, not only the target

scripts/​bash/​pre-hooks.sh:232

Fields are parsed only while $target is true, so malformed YAML scalars in any other event are silently accepted. For example, resolving before_plan succeeds when before_tasks contains extension: 'unterminated, whereas the Python resolver rejects the file during safe_load. Validate every event's fields while collecting only the requested event so invalid configuration cannot produce variant-dependent success.

Medium severity Validate non-target event entries for cross-runtime parity

scripts/​powershell/​pre-hooks.ps1:137

Non-target event entries bypass Add-HookField, so malformed quoted scalars or unsupported field values elsewhere in the same YAML file are never validated. The same file therefore fails under Python but succeeds under PowerShell depending on which event is requested. Parse/validate every event and only gate adding hooks to the result on $target.

Medium severity Standardize priority range normalization across runtimes

scripts/​python/​pre_hooks.py:63

Priority normalization has no shared numeric range across runtimes. For example, 2147483648 is retained here and by Bash but PowerShell falls back to 10; a 16-digit value is retained here while both native variants fall back. This changes hook order based on the installed script variant. Define one supported range/coercion rule in all three resolvers and cover its boundaries in the parity test.

🧠 Review effort: Balanced

Comment thread src/specify_cli/extensions/__init__.py Outdated
Reject symlinked extension configuration parents before migration, validate non-target hook fields in native and Python dispatch, and bound priority normalization across all three runtimes. Add negative, ordering, and symlink regressions.

Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 8, 2026 17:49
@mnriem

mnriem commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed this review in 219c013. Migration now rejects a symlinked .specify parent as well as a symlinked extensions.yml, without modifying the external config. Bash and PowerShell validate fields in non-target events; Python now validates every configured event too, while all three return hooks only for the requested event. Priorities share the same coercion and 1..2147483647 range, with out-of-range values falling back to 10; cross-runtime tests cover the boundaries and ordering. Validation on this exact change: LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest tests -q --tb=short — 9,966 passed, 19 skipped, 62 warnings; /bin/bash -n scripts/bash/pre-hooks.sh scripts/bash/post-hooks.sh and git diff --check passed. Posted on behalf of @mnriem by GitHub Copilot (GPT-6 Sol, autonomous agent mode, default reasoning settings); AI authored the fix, tests, documentation, and this review-round summary, with no human line-by-line review claimed.

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.

🟡 Changes recommended

The Bash resolver accepts an invalid quoted-empty identifier that the Python and PowerShell variants reject.

1 open finding
1 resolved since last review

🧠 Review effort: Balanced

Comment thread scripts/bash/pre-hooks.sh Outdated
mnriem and others added 2 commits October 8, 2026 13:00
Integrate upstream taskstoissues deprecation coverage and bundled extension version changes before fixing the PR checks.

Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Reject empty quoted identifiers in Bash for every event, update the upstream deprecation contract to assert scripted dispatch, and bump the GitHub extension and catalog to 1.0.3 so installed copies receive the command update.

Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 8, 2026 18:11

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.

🟡 Changes recommended

Shared-dispatcher lifecycle gaps and incomplete cross-event validation can bypass configured hooks.

5 open findings
1 resolved since last review

🧠 Review effort: Balanced

```
After emitting the block above you MUST actually invoke the hook and wait for it to finish before continuing. Run it the same way you would run the command yourself in this agent/session (the invocation may differ from the literal `{command}` id shown above, e.g. a skills-mode agent runs it as `/skill:speckit-...` or `$speckit-...`). Emitting the block alone does not run the hook.
- If no hooks are registered or `.specify/extensions.yml` does not exist, skip silently
Run `{PRE_HOOK_SCRIPT} taskstoissues` from the project root and read its JSON result. If it fails or returns `error`, tell the user why no hooks were checked (including mandatory hooks), then continue the core command. For each returned hook in order: invoke mandatory commands in this agent and wait for completion before proceeding; surface optional commands with their prompt and description without executing them automatically. Use the invocation syntax for the installed agent/skills mode. If `hooks` is empty, continue silently.
Comment thread scripts/bash/pre-hooks.sh Outdated
Comment thread scripts/powershell/pre-hooks.ps1 Outdated
Comment thread scripts/python/pre_hooks.py Outdated
Comment thread scripts/bash/pre-hooks.sh Outdated
Invoke Bash explicitly when Git Bash is usable and skip Bash-only cases otherwise; retain native PowerShell and Python coverage on Windows while preserving Bash coverage on macOS and Linux.

Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 8, 2026 18:36

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.

🟡 Changes recommended

Native resolvers reject supported installed-extension metadata, and generated skills lose required command-ID conversion guidance.

7 open findings
Previously missed (1)

In code that hasn't changed since last review

Medium severity Restore skill guidance for dispatcher ID-to-command mapping

tests/​integrations/​test_integration_base_skills.py:267

This replacement drops the contract that skills explain how dispatcher IDs map to installed command names. The resolver still returns dotted IDs such as speckit.git.commit, while skills are registered under hyphenated names and agent-specific prefixes. SkillsIntegration._inject_hook_command_note() (and the Cline/Junie overrides) only matches the removed - For each executable hook... wording, so the generated skills now contain no conversion guidance; these assertions mask that regression. Update the post-processing trigger for the new dispatch wording and retain coverage for slash, dollar, and /skill: invocation forms.

🧠 Review effort: Balanced

Comment thread scripts/bash/pre-hooks.sh Outdated
Comment thread scripts/powershell/pre-hooks.ps1 Outdated
Handle metadata-bearing installed entries and validate all hook identifiers across runtimes. Restore shared dispatchers on extension install/update, preserve customized copies, and reinstate skill invocation guidance with cross-runtime and lifecycle regressions.

Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 8, 2026 19:14
Preflight rejects a regular file at the projection cache path before enable, disable, or remove can change installed state. Add before-and-after regression coverage for all three commands.

Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 13, 2026 18:05

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.

🟡 Changes recommended

Projection publication has symlink, concurrency, and failed-write paths that can leave incorrect or unusable hook state.

3 open findings
1 resolved since last review

🧠 Review effort: Balanced

Comment thread src/specify_cli/extensions/__init__.py Outdated
@mnriem

mnriem commented Oct 13, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed review 5507557083 in 203b04b. A regular file at .specify/hook-dispatch now fails preflight before enable, disable, or remove can change extension state. Added public CLI regressions for all three mutations: all three failed before the fix with NotADirectoryError and now pass while verifying the registry, installed files, configuration, and colliding file remain unchanged. The focused registration and command tests passed (37 passed). The preceding commit passed both Windows pytest jobs (3.13 and 3.14); CI for this commit is newly running.

Posted on behalf of @mnriem with GitHub Copilot (GPT-6 Sol, autonomous mode); AI-authored code, tests, documentation, and review-round summary.

Check config symlinks before existence so missing targets cannot bypass CLI preflight. Cover enable, disable, remove, and direct saves while preserving installed state and projection files.

Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 13, 2026 18:25

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.

🟡 Changes recommended

Projection bounds are inconsistent across runtimes, and failed validation can still modify shared infrastructure.

3 open findings
1 resolved since last review
Previously missed (2)

In code that hasn't changed since last review

Medium severity Bound Bash projection priorities to the supported maximum

scripts/​bash/​pre-hooks.sh:295

The projection shape accepts any positive-length integer, so Bash also returns priorities above the supported 2147483647 maximum while PowerShell rejects them. Bound this field in the shape validation so a digest-consistent but semantically invalid projection fails consistently across runtimes.

Medium severity Reject priorities above the documented maximum in Python validation

scripts/​python/​pre_hooks.py:63

This validation accepts projected priorities above the documented 2147483647 maximum, unlike the PowerShell reader. A cache entry with a matching digest and priority: 2147483648 therefore succeeds in Python instead of being rejected as a semantically invalid projection, breaking cross-runtime parity.

🧠 Review effort: Balanced

Comment thread src/specify_cli/__init__.py
Reject malformed hook configuration before installing shared assets. Reject projected priorities above the supported maximum in Bash and Python, with before-and-after regression coverage for each delivery path.

Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 13, 2026 18:39

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.

🟡 Changes recommended

Legacy native parsers can silently accept null hook events, and preset lifecycle rendering can reference dispatchers that were never installed.

4 open findings
1 resolved since last review
Previously missed (1)

In code that hasn't changed since last review

Medium severity Provision dispatchers when installing or removing wrapping presets

src/​specify_cli/​integrations/​base.py:832

This substitution can produce commands that point to files that do not exist. Preset registration calls template processing and resolve_skill_placeholders directly (presets/_manager_commands.py:226 and _manager_skills.py:783) but never installs shared infrastructure. Thus, installing or removing a wrapping preset in a pre-1.1.3 project renders the new hook paths while the project still lacks the dispatchers. Provision the selected dispatchers as part of preset lifecycle/reconciliation (or fail before writing the rendered command), and cover an actual preset install starting without these files.

🧠 Review effort: Balanced

Comment thread scripts/bash/pre-hooks.sh
Comment thread scripts/powershell/pre-hooks.ps1
Reject bare hook mappings and events in the Bash and PowerShell legacy resolvers. Ensure preset install and removal have the selected hook dispatchers before rendering core command references, with command and skill lifecycle regressions.

Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 13, 2026 18:59

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.

🟡 Changes recommended

Cache publication can delete the valid snapshot before detecting a non-file collision, leaving hook dispatch unusable after a failed mutation.

3 open findings
2 resolved since last review

🧠 Review effort: Balanced

Comment thread src/specify_cli/extensions/__init__.py Outdated
Share cache-path validation between extension mutation preflight and direct saves so directory collisions fail before changing installed state or deleting the projection snapshot. Cover named and unrelated cache entries across enable, disable, remove, and direct saves.

Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 13, 2026 19:31

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.

🔵 Needs a closer look

Replace-strategy presets can incorrectly require hook dispatchers despite discarding the hook-bearing core command.

2 open findings
1 resolved since last review
Previously missed (1)

In code that hasn't changed since last review

Medium severity Provision hook dispatchers only for effective hook references

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

This provisions hook dispatchers for every command override, including the default replace strategy. A replace preset fully discards the lower core command (presets/ARCHITECTURE.md:47-56), but the helper synthesizes {CORE_TEMPLATE} and therefore sees the core hook placeholders anyway. As a result, a replacement command with no hook references can fail installation on a missing or symlinked dispatcher. Determine this from the effective rendered/composed command (and add a replace-strategy regression test) before requiring dispatchers.

🧠 Review effort: Balanced

Resolve the winning command after preset registration before checking hook dispatcher requirements. Avoid blocking hook-free replacement commands while still provisioning references in wrapping and hook-bearing replacement presets.

Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 13, 2026 19:42

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.

🟡 Changes recommended

Preset removal can provision the core script variant instead of the surviving replacement command’s required dispatcher.

3 open findings

🧠 Review effort: Balanced

Comment thread src/specify_cli/presets/_manager.py Outdated
Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 13, 2026 22:46

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.

🔵 Needs a closer look

The cross-runtime parsers and extension, preset, and integration lifecycle changes warrant final human review despite broad automated coverage.

2 open findings
1 resolved since last review

🧠 Review effort: Balanced

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-nice-to-have Verdict: evidence-backed fix or greenlit feature — land after review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants