[mnriem]

Description

Fix shared integration cleanup silently deleting a user-owned .specify/events.py.
Previously, cleanup deleted the dispatcher whenever no other installed integration
referenced it, even if the integration performing cleanup had never owned it.
This affected incoming event-capable integration setup as well as teardown.

Require ownership evidence from the integration's current or previous manifest
before deleting the last managed dispatcher. Preserve unowned, recovered, and
partially owned files even when forced, retain dispatchers referenced by another
installed integration, and preserve modified managed dispatchers with a warning
unless forced. Consult the prior manifest during empty-event upgrades so genuine
last-consumer cleanup still works with a fresh output manifest.

Thread force through event emission, teardown, and upgrades; document the event
helper/override contract. Add positive and negative ownership coverage using
existing integrations, including public install, switch, and upgrade commands.
This change contains no new integration adapter or CLI guidance changes.

Regression evidence: replaying the original cleanup function from the branch's
base commit in an isolated pytest process made all 10 unowned-dispatcher cases
fail; the corrected implementation passed those same 10 cases. The tests exercise
both incoming installation and outgoing teardown without requiring the none
integration.

Testing

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

Validation on commit 6b1316548c22fbd78c7bba74f54e57de9375d4d5:

• uv sync --extra test: passed; created this worktree's own virtual environment.
• uv run specify --help: passed, including a fresh pre-submission check.
• LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest tests/specify_cli/events tests/specify_cli/integrations/test_command_switch.py tests/specify_cli/integrations/test_command_upgrade.py tests/integrations -q: 3,290 passed, 8 skipped, 22 warnings.
• LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest tests/specify_cli/events/test_events.py tests/specify_cli/integrations/test_command_switch.py tests/specify_cli/integrations/test_command_upgrade.py -q -k 'DispatcherCleanupOwnership or DispatcherManifestClaimDroppedOnRetain or incoming_copilot_preserves_unowned_dispatcher or disabling_events_honors_modified_dispatcher_force': 28 passed, 228 deselected in the fresh pre-submission check.
• git diff --check: passed.
• Sample project: using this worktree's CLI in fresh subprocesses, ran specify init <temporary-project> --integration generic --integration-options="--commands-dir .agent/commands" --non-interactive --ignore-agent-tools, wrote # User-owned dispatcher to .specify/events.py, then ran specify integration switch copilot from that project. Both commands exited 0 and the dispatcher retained its original bytes.

The literal uv sync && uv run pytest command and the full repository suite were
not run. The targeted suites above used the documented worktree-local Python
interpreter instead of bare uv run pytest.

AI Disclosure

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

AI disclosure: Authored on behalf of @mnriem with GitHub Copilot using
GPT-6.1 Sol (gpt-6.1-sol) in autonomous/autopilot mode; reasoning effort was not
explicitly configured in the task. AI assistance covered investigation,
implementation, regression tests, documentation, validation, and this PR text.

[Copilot]

🟡 Changes recommended

Unhandled dispatcher and prior-manifest read failures can abort teardown and prevent rollback cleanup.

2 open issues

• Preserve cleanup when incoming manifest is invalid · New
• Handle unreadable dispatcher during cleanup · New

What changed in this PR

Tightens shared event-dispatcher cleanup across Specify integration lifecycles to preserve files an integration does not own.

Changes:

• Checks manifest ownership, shared references, and file modifications before deletion.
• Forwards force through event setup, teardown, and upgrades.
• Adds lifecycle regression coverage and documents the cleanup contract.

Validation used targeted standard-library checks; reported test-suite results were not independently rerun.

AI-generated review for the requester by GitHub Copilot (model: unknown; autonomous); scope: analysis and review text.

File	Description
tests/​specify_cli/​integrations/​test_command_upgrade.py	Tests forced cleanup during event-disable upgrades.
tests/​specify_cli/​integrations/​test_command_switch.py	Tests unowned-dispatcher preservation during install and switch.
tests/​specify_cli/​events/​test_events.py	Adds ownership and dispatcher-retention regressions.
src/​specify_cli/​integrations/​copilot/​__init__.py	Forwards force during event setup.
src/​specify_cli/​integrations/​command_upgrade.py	Passes force into integration setup.
src/​specify_cli/​integrations/​base.py	Propagates force through shared event lifecycle helpers.
src/​specify_cli/​events/​__init__.py	Adds ownership and hash checks before dispatcher deletion.
design/​integration.md	Documents dispatcher ownership and force behavior.

_{Review effort: Max}

[mnriem]

Addressed both findings in 6eb80d3.

Cleanup now warns and preserves the dispatcher when the prior manifest cannot be read or validated, or when non-forced dispatcher hashing encounters an I/O error. Other owned files still reach uninstall cleanup. Public install/switch regressions verify that a subsequent setup failure rolls back the generated files rather than leaving them untracked. Forced cleanup of a known managed dispatcher still skips the content read.

The focused regression selector (prior_manifest or invalid_incoming_manifest or unreadable_dispatcher) produced 10 failures and 1 pass before the fix; all 11 pass afterward.

LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest tests/specify_cli/events tests/specify_cli/integrations/test_command_install.py tests/specify_cli/integrations/test_command_switch.py tests/specify_cli/integrations/test_command_uninstall.py tests/specify_cli/integrations/test_command_upgrade.py tests/integrations/test_base.py tests/integrations/test_manifest.py -q: 499 passed, 12 warnings. git diff --check: passed.

AI disclosure: Posted on behalf of @mnriem by GitHub Copilot using GPT-6.1 Sol (gpt-6.1-sol) in autonomous/autopilot mode; reasoning effort was not explicitly configured in the task. AI assistance covered the fixes, tests, documentation, validation, and this summary. Review threads were left unresolved for the reviewer.

[Copilot]

🟡 Changes recommended

Teardown compatibility and shared-dispatcher hash handling have unresolved functional regressions.

1 open issue

• Preserve compatibility with legacy remove_events signatures · New

1 previously missed issue

In code that hasn't changed since last review

Handle stale hashes for shared generated dispatchers

src/​specify_cli/​events/​__init__.py:1762

This integration's recorded hash can be stale without any user edit. If A emitted an older dispatcher and B later emits a newer template, only B's manifest receives the new hash. Removing B retains the file for A, but subsequently removing A without force classifies those generated bytes as modified. A's claim has already been dropped, leaving an untracked dispatcher that later cleanup treats as unowned. Recognize an exact match with the current generated template, including platform newlines, after the existing ownership/reference guards, or synchronize the claiming manifests when rewriting the shared file. Add a regression covering different generated revisions.

2 resolved since last review

• Handle unreadable dispatcher during cleanup
• Preserve cleanup when incoming manifest is invalid

_{Review effort: Max}

[mnriem]

Addressed the compatibility finding and the stale shared-hash issue in 8ccdade.

Teardown now checks the bound remove_events signature before passing force. Legacy two-argument overrides remain callable, compatible overrides still receive the flag, and a TypeError raised inside an override propagates without a second invocation. Public catalog-installed adapter tests cover normal and forced uninstall and outgoing switch after unloading/reloading the adapter.

After the existing ownership and other-consumer guards, cleanup also recognizes byte-exact current generated dispatcher content with LF or CRLF endings. This permits last-consumer cleanup when another integration regenerated the dispatcher without updating the older manifest's hash. Custom edits and unowned, recovered, or partial copies remain protected. Regression coverage exercises two generated revisions, both newline styles, teardown, and empty-event upgrades.

Before the fix, the new focused cases produced 10 failures and 16 passes; afterward all 26 pass. The related event, install/switch/uninstall/upgrade, base-class, and manifest suites pass all 521 tests (12 warnings).

Commands:

• LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest tests/integrations/test_base.py tests/specify_cli/events/test_events.py tests/specify_cli/integrations/test_installed_adapters.py -q -k 'legacy_remove_events or teardown_forwards_supported_force or teardown_does_not_retry_remove_events_type_error or shared_dispatcher_revision or generated_template_does_not_establish_ownership': 26 passed.
• LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest tests/specify_cli/events tests/specify_cli/integrations/test_command_install.py tests/specify_cli/integrations/test_command_switch.py tests/specify_cli/integrations/test_command_uninstall.py tests/specify_cli/integrations/test_command_upgrade.py tests/integrations/test_base.py tests/integrations/test_manifest.py -q: 521 passed.
• git diff --check: passed.

AI disclosure: Posted on behalf of @mnriem by GitHub Copilot using GPT-6.1 Sol (gpt-6.1-sol) in autonomous/autopilot mode; reasoning effort was not explicitly configured in the task. AI assistance covered investigation, implementation, tests, documentation, validation, and this summary. Review threads remain unresolved for the reviewer.

[Copilot]

🔵 Needs a closer look

Shared-file deletion spans multiple lifecycle paths, and reported validation omits the new uninstall and installed-adapter suites.

No open issues 🎉

1 resolved since last review

• Preserve compatibility with legacy remove_events signatures

_{Review effort: Max}

[mnriem]

Validated commit 8ccdade with the complete uninstall and installed-adapter suites included, without a test-name filter. This closes the validation gap identified in review 5605585723; no implementation changes were needed for this round.

Exact command:

LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest tests/specify_cli/events tests/specify_cli/integrations/test_command_install.py tests/specify_cli/integrations/test_command_switch.py tests/specify_cli/integrations/test_command_uninstall.py tests/specify_cli/integrations/test_command_upgrade.py tests/specify_cli/integrations/test_installed_adapters.py tests/integrations/test_base.py tests/integrations/test_manifest.py -q

Result: 1,196 passed, 12 warnings in 421.86 seconds. The warnings concern existing legacy Kilocode and Bob layouts. The run covers event cleanup, public install/switch/uninstall/upgrade, the entire installed-adapter suite (including legacy-hook lifecycle regressions), base teardown, and manifest behavior. git diff --check also passed; the worktree remains clean. The full repository suite was not run.

AI disclosure: Posted on behalf of @mnriem by GitHub Copilot using GPT-6.1 Sol (gpt-6.1-sol) in autonomous mode; reasoning effort was not explicitly configured in the task. AI assistance in this round covered reading the review, running the complete related suites, and reporting validation. No code or documentation changes were made, and no review threads were resolved.

[Copilot]

🔵 Needs a closer look

Shared-file deletion spans multiple lifecycle commands and adapter overrides, warranting final human confirmation of the ownership semantics.

No open issues 🎉

_{Review effort: Max}