fix: emit required "version": 1 in .cursor/hooks.json

[sergio-sisternes-epam]

TL;DR

apm install (cursor target) was writing .cursor/hooks.json without the
top-level "version": 1 field that Cursor requires to load project hooks.
Every APM-installed hook was silently dead on arrival. This PR fixes it.

---

Problem (WHY)

Cursor's hook loader validates the project config against a schema that
requires a top-level "version" field. Without it the entire file is
rejected:

ERROR: Invalid project config: Config version must be a number
ERROR: Failed to parse project hooks configuration
No project hooks configuration found

Root cause in _integrate_merged_hooks():

json_config: dict = {}          # starts empty
if container not in json_config:
    json_config[container] = {}  # only "hooks" key is seeded
# "version" is never written

Package templates that carry "version": 1 have it stripped at parse time
because _integrate_merged_hooks() only reads the hooks map from each
source file. This affected every cursor user with hook-based packages
(APM 0.14.1 -- 0.20.0).

---

Approach (WHAT)

Add a declarative top_level_defaults field to _MergeHookConfig that
specifies target-specific top-level keys to inject when absent. Set
{"version": 1} for cursor. Inject missing defaults after seeding the
event container.

---

Implementation (HOW)

src/apm_cli/integration/hook_integrator.py

1. Add field to the dataclasses import.
2. Add top_level_defaults: dict = field(default_factory=dict) to the
   _MergeHookConfig frozen dataclass.
3. Populate top_level_defaults={"version": 1} in the cursor entry of
   _MERGE_HOOK_TARGETS.
4. In _integrate_merged_hooks(), after seeding the event container:

for key, value in config.top_level_defaults.items():
    if key not in json_config:
        json_config[key] = value

The if key not in json_config guard means reinstalling over a file
that already carries "version" is a no-op -- the user's value is
preserved.

tests/unit/integration/test_hook_integrator.py

• test_integrate_hookify_cursor: added assert config["version"] == 1
• test_cursor_version_emitted_on_fresh_install (new): fresh .cursor/
  dir produces "version": 1
• test_cursor_existing_version_preserved (new): pre-existing
  "version": 2 survives a reinstall

---

Architecture

flowchart LR
  A["_MergeHookConfig\ntop_level_defaults={'version':1}"] --> B["_integrate_merged_hooks()"]
  B --> C{key in json_config?}
  C -- No --> D["json_config[key] = value"]
  C -- Yes --> E["preserve existing value"]
  D --> F["json.dump to .cursor/hooks.json"]
  E --> F

Loading

---

Trade-offs

Decision	Rationale
top_level_defaults: dict field on _MergeHookConfig	Declarative, co-located with other target config, scales to future targets (codex, windsurf) without code changes
if key not in json_config guard	Idempotent on reinstall; preserves user-set values
Only cursor gets {"version": 1}	Other targets (claude, codex, gemini, windsurf) do not require a schema version field

---

Validation evidence

• 163 unit tests in tests/unit/integration/test_hook_integrator.py -- all pass
• ruff check -- no diagnostics
• pylint R0801 -- 10.00/10
• lint-auth-signals.sh -- clean

Fixes #1823

[Copilot]

Pull request overview

Fixes Cursor hook integration by ensuring .cursor/hooks.json always includes the required top-level "version": 1 field (without overwriting an existing user-provided version), so Cursor will actually load APM-installed project hooks.

Changes:

• Add a declarative top_level_defaults to _MergeHookConfig and apply it during merged hook integration.
• Configure the Cursor target to inject {"version": 1} when absent.
• Extend unit tests to assert version emission on fresh install and preservation on reinstall.

Reviewed changes

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

File	Description
src/apm_cli/integration/hook_integrator.py	Adds target-specific top-level defaults (Cursor "version": 1) and injects them during merge.
tests/unit/integration/test_hook_integrator.py	Adds/extends Cursor integration tests to validate emitted and preserved version.

[sergio-sisternes-epam]

APM Review Panel: ship_with_followups

Fixes silent Cursor hook breakage (v0.14.1-v0.20.0) by emitting the required top-level version:1 in .cursor/hooks.json via a declarative, idempotent defaults mechanism.

cc @sergio-sisternes-epam @danielmeppiel -- a fresh advisory pass is ready for your review.

All nine panelists converge cleanly on this PR. The fix is architecturally sound (python-architect confirms frozen-dataclass + mutable default_factory is canonical Python), security-neutral (supply-chain-security-expert confirms top_level_defaults is hardcoded and package-unreachable), and UX-correct (devx-ux-expert validates the idempotency guard mirrors npm semantics). No panelist raised a blocking-severity finding. The only material signal is a missing integration-tier assertion from test-coverage-expert, which carries an evidence block with outcome:missing on a multi-harness-support principle surface -- this is a real gap but bounded: the unit tests nail the contract, and the integration test already reads the file, so the fix is a one-line addition that does not block shipping.

The strategic signal is louder than the code signal. The oss-growth-hacker correctly identifies that six minor versions of silently dead Cursor hooks is a trust-denting event. The doc-writer independently flags the same gap: no CHANGELOG entry, no troubleshooting breadcrumb, no discoverable upgrade signal. These two panelists agree on the remedy and their recommendations reinforce each other. The CHANGELOG entry is the single highest-value follow-up because it converts a trust-repair moment into a positioning win -- APM actively names the blast radius, tells users how to repair, and demonstrates that it treats editor integrations as first-class contracts.

The cli-logging-expert and devx-ux-expert both recommend a debug-level log line on injection. This is a good pattern but genuinely optional for this PR -- the existing container-seeding path is equally silent, and adding logging to one without the other would be inconsistent. A follow-up that adds debug logging to both paths simultaneously would be cleaner.

Dissent. No substantive disagreement between panelists. The cli-logging-expert recommends debug-level logging as severity:recommended while the devx-ux-expert flags the same idea as severity:nit. I side with nit: the existing container-seeding path uses the same silent pattern, and consistency within the function matters more than traceability on a single injection site.

Aligned with: Multi-harness multi-host (Cursor is a headline integration target; this fix restores the multi-harness contract and adds a declarative mechanism that prevents the same class of bug for future targets) | Pragmatic as npm (the idempotency guard mirrors how npm seeds package.json defaults -- inject what is missing, preserve what the user set)

Growth signal. TRUST-REPAIR OPPORTUNITY -- Cursor hooks were silently dead for v0.14.1-v0.20.0. Users who hit this likely concluded APM hooks do not work in Cursor and stopped trying. The fix is clean and the declarative top_level_defaults pattern actually strengthens APM's Cursor positioning. Recommended comms: (1) CHANGELOG entry with affected version range and repair instruction (apm install --target cursor), (2) release note callout for Cursor users naming the symptom and fix, (3) consider a Discord post. Turning a silent failure into a named-and-fixed regression is a trust-building moment worth amplifying.

Panel summary

Persona	B	R	N	Takeaway
Python Architect	0	0	2	Clean, minimal fix; one nit on type annotation precision. Architecture is sound.
CLI Logging Expert	0	1	1	Silent injection is consistent with the existing container-seeding pattern. One recommended debug-log addition for verbose/agent traceability.
DevX UX Expert	0	0	3	Clean, idiomatic fix. Declarative top_level_defaults is the right pattern.
Supply Chain Security Expert	0	0	0	No supply-chain or threat-model concerns. top_level_defaults is hardcoded and package-unreachable.
OSS Growth Hacker	0	2	1	Silent-failure bug in a headline integration target for 6+ versions is trust-denting. Fix is clean. Critical gap: no CHANGELOG entry.
Doc Writer	0	2	1	PR body is accurate and complete. Two recommended gaps: missing CHANGELOG entry and troubleshooting breadcrumb.
Test Coverage Expert	0	1	0	Bug-fix regression traps present at unit tier. Integration-tier gap: version field not asserted in existing integration test.

B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.

Top 5 follow-ups

1. [Doc Writer] Add CHANGELOG entry under [Unreleased] ### Fixed with affected version range (0.14.1-0.20.0), symptom, and repair instruction -- CHANGELOG is the canonical upgrade signal. Without an entry, users on affected versions have no discoverable reason to upgrade or re-run apm install. The oss-growth-hacker independently flagged the same gap -- this is a trust-repair moment that should not ship silent.
2. [Test Coverage Expert] Add assert data.get('version') == 1 to the existing integration test test_integrates_when_cursor_dir_exists -- Evidence outcome:missing on a multi-harness-support surface. One-line addition closes the tier gap and prevents future regressions at the integration level.
3. [Doc Writer] Add troubleshooting entry in common-errors.md mapping Cursor 'Config version must be a number' to apm install --target cursor -- Users on v0.14.1-v0.20.0 whose Cursor hooks silently fail will search the docs. No breadcrumb currently exists.
4. [Python Architect] Annotate top_level_defaults as dict[str, Any] instead of bare dict -- Every other typed field on _MergeHookConfig uses precise annotations. Trivial fix, consistent with existing Any import.
5. [CLI Logging Expert] Add debug-level logging to the entire _integrate_merged_hooks flow (both container seeding and top_level_defaults injection) -- Both paths are currently silent. A follow-up covering the whole function gives --verbose users full traceability.

Architecture

classDiagram
    direction LR
    class HookIntegrator {
      <<BaseIntegrator>>
      +integrate_package_hooks_cursor(pkg_info, root)
      -_integrate_merged_hooks(config, hook_files, ...)
    }
    class _MergeHookConfig {
      <<ValueObject / frozen>>
      +config_filename str
      +target_key str
      +require_dir bool
      +schema_strict bool
      +event_container_key str
      +top_level_defaults dict
    }
    class _MERGE_HOOK_TARGETS {
      <<Registry / module-level dict>>
      +cursor _MergeHookConfig
      +claude _MergeHookConfig
      +codex _MergeHookConfig
      +gemini _MergeHookConfig
      +antigravity _MergeHookConfig
      +windsurf _MergeHookConfig
    }
    class BaseIntegrator {
      <<Abstract>>
      +files_integrated list
    }
    BaseIntegrator <|-- HookIntegrator : inherits
    HookIntegrator ..> _MERGE_HOOK_TARGETS : looks up config
    HookIntegrator ..> _MergeHookConfig : reads
    _MERGE_HOOK_TARGETS *-- _MergeHookConfig : contains

Loading

flowchart TD
    A["CLI: apm install --target cursor"] --> B["HookIntegrator.integrate_package_hooks_cursor()"]
    B --> C["Look up _MERGE_HOOK_TARGETS['cursor']"]
    C --> D["_integrate_merged_hooks(config, hook_files, ...)"]
    D --> E{"Config file exists?"}
    E -->|Yes| F["Read existing hooks.json"]
    E -->|No| G["json_config = {}"]
    F --> H["Ensure container key: json_config['hooks'] = {}"]
    G --> H
    H --> I["NEW: Inject top_level_defaults\nif 'version' not in json_config:\n  json_config['version'] = 1"]
    I --> J["For each hook_file: parse, merge into json_config['hooks']"]
    J --> K["Write hooks.json to .cursor/hooks.json"]
    style I fill:#fff3b0,stroke:#d47600

Loading

Recommendation

Ship now. Zero blocking findings across nine panelists. The fix is architecturally sound, security-neutral, and correctly idempotent. The CHANGELOG entry is the single highest-priority follow-up -- it should ideally land in this PR before merge to ensure the trust-repair narrative reaches users via the canonical upgrade channel. The integration-test assertion is a one-liner that would strengthen the regression trap. Both are small enough to fold into this PR without re-review, but neither blocks the correctness of the fix itself. Every Cursor hook user on v0.14.1-v0.20.0 is currently broken; shipping sooner is better than shipping perfect.

---

Full per-persona findings

Python Architect

• [nit] Use dict[str, Any] instead of bare dict for top_level_defaults at src/apm_cli/integration/hook_integrator.py:104
  Every other typed field on _MergeHookConfig uses precise annotations. Bare dict loses key/value type information and triggers mypy --strict warnings. dict[str, Any] is consistent with the existing Any import at line 51.
  Suggested: top_level_defaults: dict[str, Any] = field(default_factory=dict)

• [nit] Injection loop placement is correct but could benefit from a one-line comment linking it to the container init above at src/apm_cli/integration/hook_integrator.py:1406
  A single inline comment at the loop site would make the two-phase init (container + defaults) scannable without scrolling 1300 lines to the dataclass docstring.

CLI Logging Expert

• [recommended] Add _log.debug when top_level_defaults keys are injected at src/apm_cli/integration/hook_integrator.py:1413
  The injection loop silently adds missing keys, consistent with the existing container-seeding pattern. However, adding a debug log inside the if key not in json_config guard would give agents and --verbose users traceability that a field was auto-added. Zero noise for default users; helpful for debugging when a target schema changes.

• [nit] No warning emitted for unexpected existing version type at src/apm_cli/integration/hook_integrator.py
  The guard correctly skips injection when the key is already present, but emits no warning if the existing value has an unexpected type. A follow-up could add a warning for unexpected types, but this is out of scope for this fix PR.

DevX UX Expert

• [nit] Consider a DEBUG-level log line when injecting a missing top-level default at src/apm_cli/integration/hook_integrator.py
  The fix is deliberately silent on the happy path, which is correct. But when a user runs apm install --verbose to debug why hooks are not loading, a single DEBUG line would make the injection discoverable without adding noise.

• [nit] Silent failure window (0.14.1-0.20.0) suggests a missing post-write validation opportunity
  Six minor versions shipped hooks that were dead on arrival with zero APM-side feedback. Filing a follow-up: after writing hooks.json, validate that all target-required top-level fields are present and warn if not.

• [nit] The idempotency guard correctly matches reinstall expectations -- no action needed
  The if key not in json_config pattern mirrors how npm handles package.json defaults. The test_cursor_existing_version_preserved test nails this contract.

Supply Chain Security Expert

No findings.

OSS Growth Hacker

• [recommended] Missing CHANGELOG entry with affected version range (0.14.1-0.20.0) at CHANGELOG.md
  Users upgrading need a story-shaped entry naming the affected range, the symptom, and a repair instruction. This converts a trust-repair moment into a positive signal.

• [recommended] Call out affected version range explicitly in release notes at CHANGELOG.md
  Users who hit this during v0.14.1-v0.20.0 likely concluded 'APM hooks do not work in Cursor' and stopped trying. Silent failures cause permanent churn unless APM proactively names the range.

• [nit] Consider a diagnostic log line when top_level_defaults are injected on a repair path at src/apm_cli/integration/hook_integrator.py
  Turning a silent repair into a trust-building moment.

Auth Expert -- inactive

PR touches only hook_integrator.py and its tests -- no auth, token, credential, or host-classification code is affected.

Doc Writer

• [recommended] No CHANGELOG entry for a 6-version blast-radius bug fix at CHANGELOG.md
  CHANGELOG.md has no entry under [Unreleased] ### Fixed. This fix affects every Cursor user who installed a hook-bearing package with APM 0.14.1 through 0.20.0. Without an entry, users on affected versions have no discoverable signal to upgrade.
  Suggested: Add: 'apm install --target cursor now emits the required top-level version: 1 in .cursor/hooks.json. Previously (APM 0.14.1-0.20.0) the field was silently omitted, causing Cursor to reject the entire hooks config. Existing version values are preserved on reinstall. (closes [BUG] Cursor hook integration omits required top-level "version" in .cursor/hooks.json #1823) (fix: emit required "version": 1 in .cursor/hooks.json #1840)'

• [recommended] No troubleshooting entry for silent Cursor hook failure symptom at docs/src/content/docs/troubleshooting/common-errors.md
  A user on APM 0.14.1-0.20.0 whose Cursor hooks are not firing will search the docs. common-errors.md has no entry mapping Cursor's error message to apm install --target cursor as the fix.

• [nit] Pitfalls section in hooks-and-commands.md omits the now-managed Cursor version field at docs/src/content/docs/producer/author-primitives/hooks-and-commands.md
  A one-sentence note would confirm APM now injects version: 1 automatically and authors need not add it manually.

Test Coverage Expert

• [recommended] Integration test for cursor hooks does not assert version field at tests/integration/test_integrators_hooks_execution.py
TestIntegratePackageHooksCursor::test_integrates_when_cursor_dir_exists reads the generated .cursor/hooks.json and asserts hooks in data but never checks data['version'] == 1. The tier floor for hook execution is integration-with-fixtures. One-line addition closes the tier gap.
  Suggested: Add assert data.get('version') == 1 after the existing assert 'hooks' in data assertion.
  Proof (missing at integration-with-fixtures): tests/integration/test_integrators_hooks_execution.py::TestIntegratePackageHooksCursor::test_integrates_when_cursor_dir_exists -- proves: Integration-tier proof that cursor hooks.json includes required version field after apm install [multi-harness-support, devx]

Performance Expert -- inactive

PR touches only hook_integrator.py and its unit tests. No cache, deps, install pipeline, transport, or materialization paths are in the diff. O(1) dict iteration on file-write path is negligible.

_{This panel is advisory. It does not block merge. Re-apply the panel-review label after addressing feedback to re-run.}

[github-actions]

APM Review Panel: ship_with_followups

Fixes 6-version silent regression (APM 0.14.1-0.20.0) where .cursor/hooks.json missing version:1 caused Cursor to silently reject all APM-managed hooks; correct and idempotent, ship with five follow-up items.

cc @sergio-sisternes-epam @danielmeppiel -- a fresh advisory pass is ready for your review.

All active panelists agree the core fix is correct. The top_level_defaults field on _MergeHookConfig is a clean, minimal, idempotent mechanism that injects the required version:1 schema field into .cursor/hooks.json without disturbing pre-existing values. Unit-tier test evidence is solid: test_cursor_version_emitted_on_fresh_install (assert config.get("version") == 1), test_cursor_existing_version_preserved, and the amended test_integrate_hookify_cursor all pass and collectively certify the inject-and-preserve contract at tests/unit/integration/test_hook_integrator.py. No security, performance, or auth surfaces are implicated.

Three signals converge above the noise floor and deserve attention after merge. First, the integration-tier test gap: TestIntegratePackageHooksCursor::test_integrates_when_cursor_dir_exists in tests/integration/test_integrators_hooks_execution.py stops at assert "hooks" in data and never asserts data.get("version") == 1 -- a one-line fix that closes the floor-tier regression trap on the hook-execution promise; per evidence rules, a missing assertion on this surface outranks every opinion-only recommended finding in this batch. Second, three panelists (devx-ux-expert, doc-writer, oss-growth-hacker) independently flag a missing CHANGELOG entry; users upgrading from 0.14.1-0.20.0 have no signal that re-running apm install will repair their existing .cursor/hooks.json. Third, supply-chain-security raises a forward-looking type-bound concern: top_level_defaults: dict is too broad and a future contributor could silently expand the write surface; narrowing to dict[str, int | str | bool] with a __post_init__ validator is cheap and keeps the injection surface provably scalar.

The oss-growth-hacker's side-channel note is strategically accurate: Cursor is the fastest-growing AI editor segment and is prominently featured in APM's README hero line. A 6-version window of silent hook failure means a material share of early Cursor adopters received a broken first impression. The fix is the work; the CHANGELOG entry and a deliberate "Cursor hooks now work -- upgrade" callout in the next release notes are the amplifier. This cycle's highest-ROI messaging beat is the upgrade signal, and it costs one CHANGELOG line.

Dissent. supply-chain-security-expert recommended dict[str, int | str | bool] as the type bound, which is more restrictive than python-architect's and cli-logging-expert's dict[str, Any]; I side with the security expert -- top_level_defaults is scalar-only schema scaffolding and a narrow bound is both more accurate and a better guardrail against future write-surface expansion by contributors who have not read the original intent.

Aligned with: Portable by manifest, Secure by default, Multi-harness/multi-host, OSS community-driven, Pragmatic as npm

Growth signal. PR #1840 (Cursor hooks fix) is a latent retention win. Silent hook failure since 0.14.1 means a large share of early Cursor adopters got a broken first impression. The fix + a targeted "Cursor hooks now work -- upgrade" release note is the highest-ROI messaging beat available this cycle.

Panel summary

Persona	B	R	N	Takeaway
Python Architect	0	1	1	Clean minimal fix: top_level_defaults on frozen dataclass correctly injects cursor version:1 on install; sync path silently leaves {version:1} after full uninstall -- document or cover with a test.
CLI Logging Expert	0	1	2	Correct idempotent fix; inject loop (line 1416) is fully silent -- a _log.debug() on inject + preserve branches is the only logging gap.
DevX UX Expert	0	2	1	Correctness fix is solid and idempotent; two UX gaps: no CHANGELOG entry for a 6-version regression, and no in-terminal signal when the repair is applied on reinstall.
Supply Chain Security Expert	0	1	1	top_level_defaults is compile-time-only with no user/package/network input path; injection guard is correct; no prototype-pollution risk in Python. Two quality nits.
OSS Growth Hacker	0	2	1	High-impact silent regression fix for Cursor (fastest-growing AI editor). Missing CHANGELOG entry leaves 7-version user-trust story unnarrated; fix unlocks a positive 'hooks now work' story.
Doc Writer	0	1	1	CHANGELOG.md [Unreleased] has no Fixed entry for the cursor hooks.json version:1 regression; hooks-and-commands.md is otherwise accurate.
Test Coverage Expert	0	1	1	Unit tests cover version:1 regression at unit tier; existing integration-tier cursor test not updated to assert version, leaving floor-tier gap (one-line fix).

B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.

Top 5 follow-ups

1. [Test Coverage Expert] Add assert data.get('version') == 1 in TestIntegratePackageHooksCursor::test_integrates_when_cursor_dir_exists (tests/integration/test_integrators_hooks_execution.py) -- Missing integration-tier assertion on the hook-execution promise is the highest-signal gap per evidence rules; the unit tests prove the injector is correct but the integration floor would not catch a future regression at the wiring layer. One line closes the trap.
2. [DevX UX Expert] Add CHANGELOG.md [Unreleased] ### Fixed entry for the cursor hooks.json version:1 regression with a reinstall call-to-action -- Three panelists (devx-ux-expert, doc-writer, oss-growth-hacker) converge on this gap; without a CHANGELOG line, users upgrading from 0.14.1-0.20.0 have no signal that re-running apm install is required to repair existing deployments. Must land before next release cut.
3. [Supply Chain Security Expert] Narrow top_level_defaults field type to dict[str, int | str | bool] and add a __post_init__ validator rejecting non-scalar values -- Prevents silent write-surface expansion by future contributors adding nested structures; cheap static guardrail with no runtime overhead on the install hot path. Resolves the type-annotation dissent between security and architect personas in favor of the narrower bound.
4. [CLI Logging Expert] Add _log.debug('target %s: injected missing top-level key %r=%r', config.target_key, key, value) inside the if key not in json_config branch (line ~1416) -- Two panelists (cli-logging-expert, devx-ux-expert) converge; the original breakage was silent and the repair is also silent. A verbose-mode user debugging still-broken hooks cannot confirm the fix fired without a trace line.
5. [Python Architect] Document or test the sync-path asymmetry: version:1 is injected on install but not reclaimed by _clean_apm_entries_from_json on full uninstall, leaving {"version":1} residual in .cursor/hooks.json -- Not a correctness regression (Cursor parses the residual as a valid empty config) but the asymmetry is unintentional-looking to a user running git status after apm uninstall. A comment in _clean_apm_entries_from_json or a test asserting the expected residual state makes the behavior deliberate rather than accidental.

Architecture

classDiagram
    direction LR
    class _MergeHookConfig:::touched {
        <<ValueObject>>
        +config_filename: str
        +target_key: str
        +require_dir: bool
        +schema_strict: bool
        +event_container_key: str
        +top_level_defaults: dict
    }
    class _MERGE_HOOK_TARGETS {
        <<Registry>>
        +cursor _MergeHookConfig
        +claude _MergeHookConfig
        +codex _MergeHookConfig
        +gemini _MergeHookConfig
        +windsurf _MergeHookConfig
        +antigravity _MergeHookConfig
    }
    class HookIntegrator {
        <<Service>>
        +integrate_package_hooks_cursor()
        +integrate_hooks_for_target()
        +sync_integration()
        -_integrate_merged_hooks()
        -_clean_apm_entries_from_json()
    }
    class BaseIntegrator {
        <<AbstractBase>>
        +check_collision()
        +try_adopt_identical()
        +cleanup_empty_parents()
    }
    class HookIntegrationResult {
        <<ValueObject>>
        +files_integrated: int
        +scripts_copied: int
    }
    class IntegrationResult {
        <<ValueObject>>
        +files_integrated: int
        +files_updated: int
        +target_paths: list
    }
    BaseIntegrator <|-- HookIntegrator
    IntegrationResult <|-- HookIntegrationResult
    HookIntegrator ..> _MERGE_HOOK_TARGETS : lookups config by target key
    _MERGE_HOOK_TARGETS o-- _MergeHookConfig : contains
    HookIntegrator ..> HookIntegrationResult : returns
    note for _MergeHookConfig "top_level_defaults: injects target-required schema fields absent from file; e.g. cursor version=1"
    classDef touched fill:#fff3b0,stroke:#d47600
    class _MergeHookConfig:::touched

Loading

flowchart TD
    A["integrate_package_hooks_cursor"] --> B
    B["_integrate_merged_hooks"] --> C
    C["find_hook_files + filter for cursor target"] --> D
    D{"hook_files empty?"}
    D -- yes --> E["return HookIntegrationResult files_integrated=0"]
    D -- no --> F["read .cursor/hooks.json\njson_config={} on miss or decode-error"]
    F --> G["seed event container:\nif hooks not in json_config: json_config[hooks]={}"]
    G --> H["NEW: inject top_level_defaults\nfor k,v in config.top_level_defaults.items()\nif k not in json_config: json_config[k]=v\ncursor: sets version=1 when absent"]
    H --> I["for each hook_file:\nparse + rewrite hooks data\nmerge entries into json_config[hooks]"]
    I --> J["json.dump json_config to .cursor/hooks.json"]
    J --> K["return HookIntegrationResult"]

Loading

Recommendation

The fix is correct, idempotent, and well-covered at unit tier; the panel is unanimous on the core implementation and no panelist raised a blocking-severity finding. Ship the merge and track five follow-up items as a post-merge batch: the integration-tier assertion (one line, closes the regression trap), the CHANGELOG entry (must land before next release cut to drive upgrades for the Cursor user base), the type-bound narrowing, the _log.debug trace, and the sync-path asymmetry note. None of these warrant holding a regression fix that unblocks all Cursor hook users on six affected versions.

---

Full per-persona findings

Python Architect

• [recommended] Sync path has no knowledge of top_level_defaults; full Cursor uninstall leaves {"version":1} ghost in hooks.json at src/apm_cli/integration/hook_integrator.py:1970
  _clean_apm_entries_from_json removes APM-tagged hook entries and deletes the event container key when empty, but never touches keys injected via top_level_defaults. After a full install+uninstall cycle, version:1 is abandoned in .cursor/hooks.json. Cursor parses this without error, so there is no correctness regression, but APM is asymmetric: it injects on install and never reclaims on uninstall.
  Suggested: Option A: add a comment in _clean_apm_entries_from_json noting that top_level_defaults-injected keys are deliberately left in place (schema scaffolding, not APM content markers). Option B: add test_cursor_version_after_full_uninstall asserting the expected residual state.
• [nit] top_level_defaults uses bare dict annotation; dict[str, Any] is more precise at src/apm_cli/integration/hook_integrator.py:107
  Any is already imported at L52. The bare dict annotation conveys nothing about the expected key/value types.
  Suggested: top_level_defaults: dict[str, Any] = field(default_factory=dict)

CLI Logging Expert

• [recommended] Inject loop emits no trace in any log level -- verbose agents cannot confirm version:1 was written at src/apm_cli/integration/hook_integrator.py:1416
  Lines 1416-1418 silently mutate json_config with no _log.debug() call. An agent running apm install --verbose on a fresh Cursor project has no way to observe that the top-level defaults injection fired. This matters especially for debugging future schema regressions.
  Suggested: _log.debug('target %s: injected missing top-level key %r=%r', config.target_key, key, value) inside the if key not in json_config branch.
• [nit] Type annotation uses bare dict instead of dict[str, Any] at src/apm_cli/integration/hook_integrator.py:107
  The file already imports Any from typing and uses dict[str, Any] elsewhere.
  Suggested: top_level_defaults: dict[str, Any] = field(default_factory=dict)
• [nit] No migration hint for pre-existing hooks.json files missing version:1 at src/apm_cli/integration/hook_integrator.py:1374
  Users who installed Cursor hooks before this fix have a broken .cursor/hooks.json on disk. The fix only applies on next install/reinstall with no APM-visible signal.
  Suggested: After reading an existing json_config, check if any top_level_defaults key is absent and emit _rich_warning with a reinstall recommendation.

DevX UX Expert

• [recommended] No CHANGELOG entry for a 6-version regression affecting all cursor hook users at CHANGELOG.md
  All cursor users with hook-based packages had silently dead hooks from 0.14.1 through 0.20.0. Without a CHANGELOG line, users cannot discover the fix and do not know they need to reinstall.
  Suggested: Add under [Unreleased] ### Fixed: "apm install --target cursor now emits the required top-level "version": 1 in .cursor/hooks.json; re-run apm install to repair existing deployments."
• [recommended] Silent repair: no user-facing output when version:1 is injected into an existing file at src/apm_cli/integration/hook_integrator.py
  The original breakage was silent and the fix is also silent. A user who suspects their hooks were broken and runs apm install to repair gets zero in-terminal confirmation that anything changed.
  Suggested: Emit a _rich_info line when top_level_defaults are actually injected (gate on if key not in json_config so it only fires on repair, not on idempotent reinstalls).
• [nit] No _log.debug trace when top_level_defaults are injected, hampering verbose-mode diagnosis at src/apm_cli/integration/hook_integrator.py
  Suggested: _log.debug("target %s: injected top-level default %r = %r", config.target_key, key, value)

Supply Chain Security Expert

• [recommended] Unparameterized dict type for top_level_defaults provides no static guardrail against future callers injecting complex/mutable values at src/apm_cli/integration/hook_integrator.py
  A future contributor could add a nested list or dict (e.g. top_level_defaults={'hooks': [...]}) without a type-checker warning. The pattern is being established as a reusable field; bounding the type now is cheap and prevents silent expansion of the write surface.
  Suggested: Change to dict[str, int | str | bool] to bound injectable scalar types; add a __post_init__ validator rejecting non-scalar values.
• [nit] frozen=True does not protect the dict contents; top_level_defaults is mutable at runtime despite the frozen dataclass at src/apm_cli/integration/hook_integrator.py
_MERGE_HOOK_TARGETS objects are shared module-level singletons; any code with a reference can mutate top_level_defaults mid-run without raising an error.
  Suggested: Wrap the dict in types.MappingProxyType at construction time.

OSS Growth Hacker

• [recommended] No CHANGELOG entry in [Unreleased] -- the 7-version Cursor regression story goes untold
  Cursor is prominently listed in the README hero line. Without a CHANGELOG entry, the regression never becomes a 'now fixed' story and existing users have no upgrade signal.
  Suggested: Add to CHANGELOG.md [Unreleased] ### Fixed with version range (0.14.1-0.20.0) and reinstall CTA.
• [recommended] Release notes for the next version should carry an explicit 'Cursor hooks now work -- upgrade' callout
  Cursor users who gave up on APM hooks after seeing them silently fail will not re-test unless there is a deliberate signal in the release post naming the exact symptom and the fixing version.
  Suggested: Lead the Fixed section with a Cursor-specific callout: 'Cursor hooks: if you installed APM hook packages in 0.14.1-0.20.0 and never saw them run, upgrade now.'
• [nit] The top_level_defaults extensibility story is buried in a PR trade-offs table -- surface it for contributors
  Adding hook support for a new AI editor that needs a schema version field is now a one-liner in _MERGE_HOOK_TARGETS. That lowers the bar for community PRs.
  Suggested: Add a one-liner in CONTRIBUTING.md or docs: 'To add top-level schema fields for a new hook target, set top_level_defaults in _MERGE_HOOK_TARGETS -- no code changes required.'

Doc Writer

• [recommended] CHANGELOG.md [Unreleased] is missing a Fixed entry for the cursor hooks.json version:1 regression at CHANGELOG.md
  Users upgrading from 0.14.1-0.20.0 need the changelog to tell them what changed and that re-running apm install is required to fix deployed configs.
  Suggested: Under [Unreleased] ### Fixed: "apm install --target cursor now emits the required top-level "version": 1 in .cursor/hooks.json; the field was absent since 0.14.1, causing Cursor to silently reject every APM-installed hook. Re-run apm install to repair existing deployments. (closes [BUG] Cursor hook integration omits required top-level "version" in .cursor/hooks.json #1823) (fix: emit required "version": 1 in .cursor/hooks.json #1840)"
• [nit] hooks-and-commands.md Pitfalls section could note that Cursor requires top-level version:1 and that APM injects it at docs/src/content/docs/producer/author-primitives/hooks-and-commands.md
  A package author auditing hooks.json by hand could remove the version field, re-introducing the bug.
  Suggested: In Pitfalls: 'Cursor hooks.json schema: Cursor requires a top-level "version": 1 in .cursor/hooks.json. APM injects it automatically; do not add or remove it by hand.'

Test Coverage Expert

• [recommended] Integration-tier cursor hook test not updated to assert version:1; hook-execution promise uncertified at floor tier at tests/integration/test_integrators_hooks_execution.py
TestIntegratePackageHooksCursor::test_integrates_when_cursor_dir_exists asserts result.hooks_integrated >= 1 and 'hooks' in data but lacks assert data.get('version') == 1. This is the integration floor for the hook-execution surface.
  Suggested: Add assert data.get('version') == 1 immediately after assert 'hooks' in data.
  Proof (missing at integration-with-fixtures): tests/integration/test_integrators_hooks_execution.py::TestIntegratePackageHooksCursor::test_integrates_when_cursor_dir_exists -- proves: apm install on a project with an existing .cursor dir writes version:1 to .cursor/hooks.json so Cursor loads the hooks [devx, portability-by-manifest]
  assert data.get("version") == 1 -- absent; test currently ends at: assert "hooks" in data
• [nit] Unit-tier coverage: three cursor tests correctly assert version:1 and idempotency guard at tests/unit/integration/test_hook_integrator.py
test_cursor_version_emitted_on_fresh_install, test_cursor_existing_version_preserved (new) and the amended test_integrate_hookify_cursor correctly cover the fix at unit tier. Unit evidence is necessary but not sufficient for the hook-execution surface.
  Proof (passed at unit): tests/unit/integration/test_hook_integrator.py::test_cursor_version_emitted_on_fresh_install -- proves: HookIntegrator.integrate_package_hooks_cursor() injects version:1 into a fresh .cursor/hooks.json and preserves any pre-existing version value
  assert config.get("version") == 1

Auth Expert -- inactive

PR #1840 only modifies src/apm_cli/integration/hook_integrator.py and tests/unit/integration/test_hook_integrator.py (hook config generation), which touch no authentication, token management, credential resolution, host classification, or remote-host fallback surfaces.

Performance Expert -- inactive

Both changed files are in the hook-integration layer, not on any download, materialization, cache, or resolve hot path; the only runtime delta is a single dict iteration over a one-entry top_level_defaults dict per hook-integration call.

_{This panel is advisory. It does not block merge. Re-apply the
panel-review label after addressing feedback to re-run.}

Generated by PR Review Panel for issue #1840 · sonnet46 11.5M · ◷