You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
PR 1 of the staged #4883 split. Publish a validated, digest-indexed .specify/hook-dispatch/ projection only when a project opts in with the literal JSON boolean "native_extension_hooks": true in .specify/init-options.json. With the key absent or false, existing markdown/YAML hooks (including legacy event names) retain their original behavior and no projection is produced; other key types fail explicitly. An opt-out retires an existing cache on configuration save or shared-infrastructure refresh. No dispatch scripts, placeholder provisioning, preset changes, core templates, or bundled extension behavior are changed in this PR.
For opted-in projects, the producer filters disabled and conditional hooks, retains YAML declaration order (including mandatory/optional metadata), and publishes one source-matched JSON/digest per event. Event names must use lowercase letters, digits, and underscores in before_<target> or after_<target> form; extension-owned targets use an underscore-normalized target namespace (for example, before_bug_assess). These stricter names apply only when opted in: legacy names such as custom:after or hyphenated names continue to work by default but fail with migration guidance when opted in, rather than being silently rewritten or dropped. Opted-in installs preserve manifest enabled: false in the hook registry and exclude those hooks from the projection; opt-out retains its prior registration behavior. Opted-in disable/enable cycles now restore each owned hook's prior effective enabled value instead of activating declaration-disabled or user-disabled hooks; older disabled installations lacking saved state use their installed manifest declarations. Without opt-in, legacy enable still activates all owned hooks. Preflight rejects malformed source and unsafe paths before lifecycle mutations. Refresh derives JSON from the current YAML bytes, preserving comments and CRLF in source.yml.
User-directed tradeoff for review 5522542843: Opt-in projection publication writes YAML and cache directly rather than locking, staging, or restoring an exact prior pair. A write failure reports the error and manual recovery guidance; it may leave incomplete YAML/cache. Fix the filesystem problem, repair .specify/extensions.yml if needed, and retry the extension operation or run specify integration switch <current-key> --refresh-shared-infra. Git cannot recover uncommitted changes. Failed install publication no longer runs a destructive same-ID unregister_hooks compensation, but a partial direct write is not guaranteed to preserve prior bytes. Removal cleans up artifacts/sources before unregistering hooks, so a normal cleanup failure does not mutate hook YAML/cache; failure during hook publication can still leave partially removed artifacts and hook files. This deliberately declines automatic exact-byte rollback, not a claim of fully atomic extension removal. Root AGENTS.md documents the general writable-project, single-mutating-CLI operational assumption without changing unrelated commands. Both disable and enable lifecycle transactions (registry, generic invocation artifacts, hook configuration/publication, and failure rollback) now belong to ExtensionManager.disable() and ExtensionManager.enable(); CLI handlers resolve the extension and present results/errors. Current installed command dispatch remains unchanged.
Baseline comparison: This branch's merge base with github/spec-kit main is 51b6d170aa7368205bee629d75a7059185272d19. A separate upstream archive and this branch produced identical HookExecutor.check_hooks_for_event('before_plan') order and formatted message on the same optional, mandatory, priority, and condition fixture. Only opted-in projects create the new projection; default installed commands keep their original read path. Upstream collected 12,374 tests; this branch collects 12,466 (none removed). The direct-write regressions failed against prior local staged commit 1081bc03 (two did not raise on injected live-write failures; removal returned success instead of reporting a live-index write failure) and pass on 954998ad. The new end-to-end enable regression failed against previous PR head 954998ad ([True, True] rather than [False, True]) and passes on 60b2456a.
Testing
Tested locally with uv run specify --help — passed on head 60b2456a.
Ran existing tests with uv sync && uv run pytest — used the worktree-local interpreter instead, as required by this repository's editable-install guidance: uv sync --extra test --quiet passed earlier (no dependency changes); LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest -q --ignore=tests/specify_cli/integrations/test_installed_adapters.py passed (11,699 passed, 98 skipped); LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest -q tests/specify_cli/integrations/test_installed_adapters.py passed (669 passed). I ran these disjoint partitions on the final worktree before committing 60b2456a0d868ee520b31f9ebae915ef7dd8dcf1 without subsequent code changes; together they collected 12,466 tests.
Tested with a sample project (if applicable) — CLI add on a temporary opted-in project verifies source/payload and unchanged rendered command output; default add/update and refresh tests verify legacy names remain accepted without a cache. Failure-injection tests cover install, remove (including partial cleanup and direct-write recovery), disable/enable rollback, publication, and refresh consistency. The final committed extension suite passed (274 passed) with opted-in lifecycle, manifest-disabled, user-override, and default opt-out coverage.
git diff --check upstream/main...HEAD passed on the final head.
AI Disclosure
I did not use AI assistance for this contribution
I did use AI assistance (fill in the disclosure below)
AI disclosure: GitHub Copilot App, GPT-6 Sol, default reasoning setting (not explicitly overridden), autonomous. Generated and revised implementation, tests, and documentation; ran automated and observational comparisons. No human line-by-line review is claimed.
Record newly created hook lock files in the integration lifecycle journal so a failed install restores the prior project snapshot.
Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5f460283-8c52-47f4-8cfe-f67a833296cf
Keep registration regression coverage for corrupted configuration while asserting that publication now fails explicitly and preserves the original YAML.
Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5f460283-8c52-47f4-8cfe-f67a833296cf
On behalf of @mnriem: review-round update for commit 66de016240cfc6280d2b3d50a839b7c0566dc101. Manifest hook fields now validate before installation; publication reads the existing config before creating a staging directory, and failed post-commit backup cleanup is logged without misreporting the committed update as a failed operation. Added CLI regression tests for add, enable, disable, and remove, including generic invocation-artifact rollback. Validation: 12,410 collected tests; 11,643 passed and 98 skipped in the non-adapter suite, and 669 passed in the adapter suite.
AI disclosure: GitHub Copilot App, GPT-6 Sol, default reasoning setting (not explicitly overridden), autonomous. The agent generated the implementation, tests, and this review-round summary; no human line-by-line review is claimed.
In refresh mode, projected was computed before this lock from the caller's earlier parse, while source is re-read here after acquiring it. A concurrent CLI save can therefore publish config B, then this refresh installs event JSON derived from config A alongside source.yml copied from B. Parse the source bytes and derive the projection from that exact snapshot while holding the same lock (or hold the lock across load and publish) so the published pair cannot disagree.
Propagate failures during enabled extension restoration
register_enabled_extensions_for_agent() is explicitly best-effort: it catches each extension failure, warns, and returns (__init__.py:4282-4311). If artifact recreation fails here, this rollback still appears successful and leaves the registry enabled while the generic invocation artifact remains absent. Use a restoration path that propagates failure, or verify this extension's expected artifacts/metadata after registration and report rollback failure.
Preserve legacy markdown hook behavior by default; opt in to strict event projection with native_extension_hooks. Keep source and projection synchronized on refresh, roll back installation/removal publication failures, and surface unsuccessful artifact restoration.
Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5f460283-8c52-47f4-8cfe-f67a833296cf
Opted-in installs validate a manifest hook's enabled field here, but register_hooks() later persists every entry with "enabled": True (line 6020). A manifest hook declared with enabled: false therefore passes preflight and is still emitted into the projection, contradicting the documented “only enabled hooks” contract. Preserve the manifest value when constructing the stored entry and add an install-path regression test for a disabled manifest hook.
Keep the CLI responsible only for resolution, output and error mapping; preserve generic rollback and opt-out behavior with direct operation tests.
Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5f460283-8c52-47f4-8cfe-f67a833296cf
On behalf of @mnriem: review 5521674125 is addressed in commits 7d33e9126012ebc49fa3db9eeb63a98846c8064c and 9d7f9cadebbf707df1af7d26e9a302574f2410f5 (current head). Opted-in removal restores the prior hook YAML and projection after downstream cleanup failures or reports an explicit rollback failure; this does not make command-artifact or source cleanup atomic. Manifest enabled: false remains disabled in opted-in installs and is excluded from projection, while opt-out retains the earlier behavior. The documentation qualifies canonical event naming as opt-in only. The entire disable transaction now belongs to ExtensionManager.disable(), with the CLI limited to resolution and presentation.
Regression tests failed against the preceding implementations and pass with these fixes. Validation on the final tree: LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest -q --ignore=tests/specify_cli/integrations/test_installed_adapters.py passed (11,684 passed, 98 skipped); LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest -q tests/specify_cli/integrations/test_installed_adapters.py passed (669 passed). The disjoint partitions collect 12,451 tests; focused extension lifecycle suites passed (108 passed).
AI disclosure: GitHub Copilot App, GPT-6 Sol, default reasoning setting (not explicitly overridden), autonomous, acting on behalf of @mnriem. The agent generated/revised implementation, tests, documentation, and this comment, and ran the automated comparisons and tests. No human line-by-line review is claimed.
Use an expected-state check at the projection commit and compare the committed pair during compensation; report conflicts instead of overwriting newer hook data.
Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5f460283-8c52-47f4-8cfe-f67a833296cf
Keep the two reviewed opt-in lifecycle rollback fixes without adding publisher CAS or multi-process conflict handling.
Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5f460283-8c52-47f4-8cfe-f67a833296cf
Review 5522542843 follow-up on 954998adea65b43c6e61c8208962abd119e835bc: the user chose direct writes under a writable-project, single-mutating-CLI assumption instead of projection-specific locks, staging, or automatic byte-exact restoration. I removed the destructive same-ID unregister compensation after failed opted-in install publication and moved hook unregister after artifact/source cleanup on removal. A partial direct write can still leave incomplete YAML/cache or partially removed artifacts; the CLI reports the I/O error and directs manual repair followed by retry or shared-infrastructure refresh. This does not claim the requested automatic restoration or fully atomic removal. Root AGENTS.md documents the operational assumption. Three direct-write regressions failed on the previous staged implementation and pass on the new commit; the extension suite passed 266 tests, and the two disjoint full-suite partitions passed 11,691 tests (98 skipped) and 669 adapter tests, with 12,458 total collected. On behalf of @mnriem: this summary and the implementation, tests, and docs were authored/revised with GitHub Copilot App, GPT-6 Sol, default reasoning setting (not explicitly overridden), autonomous; automated and observational comparisons were run. No human line-by-line review is claimed.
Disabled manifest hooks are re-enabled after lifecycle cycling
src/specify_cli/extensions/__init__.py:6100
Persisting the manifest's enabled: false here is undone by the extension enable lifecycle: extension disable writes every owned hook to false, and extension enable then writes every owned hook to true (command_enable.py:92-98). Consequently, a hook declared disabled becomes active and enters the projection after one disable/enable cycle, so the promised manifest-disabled state is not preserved. Keep declaration-level enablement separate from the extension's lifecycle state (or otherwise restore each hook's prior value) and add a lifecycle regression covering an initially disabled hook.
Move enable transaction orchestration from CLI handler to domain layer
This added block makes the Typer handler own state-dependent validation, hook publication, and rollback across the registry and generic artifacts. That leaves enable without a domain operation equivalent to the new ExtensionManager.disable() and conflicts with design/cli.md:57-69 and design/shared.md:68-84, which assign semantic orchestration and side effects below the CLI adapter. Move the enable transaction into ExtensionManager.enable() (or a shared operation) and leave this handler to resolution, rendering, and exit-code mapping.
Move the enable lifecycle transaction into ExtensionManager, recording each owned hook enabled value on disable and restoring it on re-enable without changing legacy opt-out behavior. User-directed review 5523638830 follow-up.
Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5f460283-8c52-47f4-8cfe-f67a833296cf
Review 5523638830 follow-up on 60b2456a0d868ee520b31f9ebae915ef7dd8dcf1: opted-in disable/enable now records and restores each owned hook’s effective enabled state, so declaration-disabled and user-disabled hooks stay out of the projection; default opt-out retains its legacy enable-all behavior. The registry, generic-invocation, hook-publication, and rollback semantics of enable moved from the CLI into ExtensionManager.enable(). The end-to-end regression failed on the previous PR head ([True, True] instead of [False, True]) and passes now; 274 extension tests passed on the committed head. I ran the two disjoint full-suite partitions on the identical final worktree: 11,699 passed / 98 skipped, and 669 adapter tests passed (12,466 collected). The repeated low test findings already have coverage: test_remove_publication_failure_reports_partial_cleanup_and_recovery checks remove error mapping; test_enable_publication_failure_restores_generic_disabled_state and test_domain_enable_publication_failure_retains_disabled_hook_state check enable rollback; test_direct_cache_write_failure_leaves_partial_state_then_refresh_repairs checks projection failure/recovery. The custom:after decision remains an intentional opt-in-only restriction, with default manifests unaffected. On behalf of @mnriem: this comment, implementation, tests, and documentation were authored/revised with GitHub Copilot App (GPT-6 Sol, default reasoning setting not explicitly overridden, autonomous); I ran automated and observational comparisons. No human line-by-line review is claimed.
Retire stale hook-dispatch projection when extensions are removed
src/specify_cli/__init__.py:195
Shared-infrastructure refresh only runs migration when extensions.yml exists. If a prior projection remains after that file was deleted and the project opts out, refresh leaves the stale .specify/hook-dispatch/ directory in place instead of retiring it. Include an existing projection path in this post-refresh condition.
Refresh projection even when native configuration is unchanged
src/specify_cli/extensions/__init__.py:6178
This native one-write path can complete without publishing when the existing YAML already equals the manifest-derived state: changed remains false and the later save is skipped. A successful install against stranded or pre-existing same-ID configuration can therefore leave .specify/hook-dispatch/ absent or stale. The unchanged native path should refresh the projection while preserving the current YAML bytes.
if isinstance(entry, dict) and entry.get("command")
]
This branch has not been deployed
No deployments
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
triage-must-haveVerdict: high-value, important work for Spec Kit — do first
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
PR 1 of the staged #4883 split. Publish a validated, digest-indexed
.specify/hook-dispatch/projection only when a project opts in with the literal JSON boolean"native_extension_hooks": truein.specify/init-options.json. With the key absent orfalse, existing markdown/YAML hooks (including legacy event names) retain their original behavior and no projection is produced; other key types fail explicitly. An opt-out retires an existing cache on configuration save or shared-infrastructure refresh. No dispatch scripts, placeholder provisioning, preset changes, core templates, or bundled extension behavior are changed in this PR.For opted-in projects, the producer filters disabled and conditional hooks, retains YAML declaration order (including mandatory/optional metadata), and publishes one source-matched JSON/digest per event. Event names must use lowercase letters, digits, and underscores in
before_<target>orafter_<target>form; extension-owned targets use an underscore-normalized target namespace (for example,before_bug_assess). These stricter names apply only when opted in: legacy names such ascustom:afteror hyphenated names continue to work by default but fail with migration guidance when opted in, rather than being silently rewritten or dropped. Opted-in installs preserve manifestenabled: falsein the hook registry and exclude those hooks from the projection; opt-out retains its prior registration behavior. Opted-in disable/enable cycles now restore each owned hook's prior effective enabled value instead of activating declaration-disabled or user-disabled hooks; older disabled installations lacking saved state use their installed manifest declarations. Without opt-in, legacy enable still activates all owned hooks. Preflight rejects malformed source and unsafe paths before lifecycle mutations. Refresh derives JSON from the current YAML bytes, preserving comments and CRLF insource.yml.User-directed tradeoff for review 5522542843: Opt-in projection publication writes YAML and cache directly rather than locking, staging, or restoring an exact prior pair. A write failure reports the error and manual recovery guidance; it may leave incomplete YAML/cache. Fix the filesystem problem, repair
.specify/extensions.ymlif needed, and retry the extension operation or runspecify integration switch <current-key> --refresh-shared-infra. Git cannot recover uncommitted changes. Failed install publication no longer runs a destructive same-IDunregister_hookscompensation, but a partial direct write is not guaranteed to preserve prior bytes. Removal cleans up artifacts/sources before unregistering hooks, so a normal cleanup failure does not mutate hook YAML/cache; failure during hook publication can still leave partially removed artifacts and hook files. This deliberately declines automatic exact-byte rollback, not a claim of fully atomic extension removal. RootAGENTS.mddocuments the general writable-project, single-mutating-CLI operational assumption without changing unrelated commands. Both disable and enable lifecycle transactions (registry, generic invocation artifacts, hook configuration/publication, and failure rollback) now belong toExtensionManager.disable()andExtensionManager.enable(); CLI handlers resolve the extension and present results/errors. Current installed command dispatch remains unchanged.Baseline comparison: This branch's merge base with
github/spec-kitmain is51b6d170aa7368205bee629d75a7059185272d19. A separate upstream archive and this branch produced identicalHookExecutor.check_hooks_for_event('before_plan')order and formatted message on the same optional, mandatory, priority, and condition fixture. Only opted-in projects create the new projection; default installed commands keep their original read path. Upstream collected 12,374 tests; this branch collects 12,466 (none removed). The direct-write regressions failed against prior local staged commit1081bc03(two did not raise on injected live-write failures; removal returned success instead of reporting a live-index write failure) and pass on954998ad. The new end-to-end enable regression failed against previous PR head954998ad([True, True]rather than[False, True]) and passes on60b2456a.Testing
uv run specify --help— passed on head60b2456a.uv sync && uv run pytest— used the worktree-local interpreter instead, as required by this repository's editable-install guidance:uv sync --extra test --quietpassed earlier (no dependency changes);LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest -q --ignore=tests/specify_cli/integrations/test_installed_adapters.pypassed (11,699 passed, 98 skipped);LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest -q tests/specify_cli/integrations/test_installed_adapters.pypassed (669 passed). I ran these disjoint partitions on the final worktree before committing60b2456a0d868ee520b31f9ebae915ef7dd8dcf1without subsequent code changes; together they collected 12,466 tests.git diff --check upstream/main...HEADpassed on the final head.AI Disclosure
AI disclosure: GitHub Copilot App, GPT-6 Sol, default reasoning setting (not explicitly overridden), autonomous. Generated and revised implementation, tests, and documentation; ran automated and observational comparisons. No human line-by-line review is claimed.