diff --git a/.apm/architecture/owners/hooks-integrations.json b/.apm/architecture/owners/hooks-integrations.json index 588f42457..a2e0dbd9a 100644 --- a/.apm/architecture/owners/hooks-integrations.json +++ b/.apm/architecture/owners/hooks-integrations.json @@ -45,6 +45,7 @@ "src/apm_cli/hook_contract.py", "src/apm_cli/integration/hook_ir.py", "src/apm_cli/integration/hook_native_formats.py", + "src/apm_cli/integration/hook_cursor_preflight.py", "src/apm_cli/integration/hook_integrator.py", "src/apm_cli/integration/hook_ownership.py" ], diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b87fefe9..beb4e5300 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Codex agent conversion preserves native `model` and `model_reasoning_effort` settings and warns about dropped metadata instead of silently losing it. (#3150) +- Cursor hooks use native v1 events and flat handlers; unsupported input and overlapping Claude imports now fail explicitly. Choose one hook route per dependency; see the [supported mappings](docs/src/content/docs/producer/author-primitives/hooks-and-commands.md#cursor-native-hooks-and-claude-import) and `openapm-v0.1.md` requirements `req-tg-016/017`. (#3149) ## [0.33.0] - 2026-10-02 diff --git a/CONFORMANCE.json b/CONFORMANCE.json index c5e719ae2..8705234bb 100644 --- a/CONFORMANCE.json +++ b/CONFORMANCE.json @@ -302,9 +302,10 @@ "keyword": "MUST", "section": "5.2", "status": "active", - "test_count": 2, + "test_count": 3, "tests": [ "tests/spec_conformance/test_lockfile_reqs.py::test_cli_target_without_manifest_targets_preserves_inactive_state", + "tests/spec_conformance/test_lockfile_reqs.py::test_dropped_cursor_target_merge_hook_state_reconciled_fail_safe", "tests/spec_conformance/test_lockfile_reqs.py::test_dropped_target_merge_hook_state_reconciled_fail_safe" ] }, @@ -1555,12 +1556,37 @@ "tests": [ "tests/spec_conformance/test_manifest_reqs.py::test_codex_native_model_settings_preserved_and_dropped_metadata_bounded" ] + }, + { + "conformance_class": "consumer", + "id": "req-tg-016", + "keyword": "MUST", + "section": "8.5.9", + "status": "active", + "test_count": 2, + "tests": [ + "tests/spec_conformance/test_cursor_hook_reqs.py::test_cursor_native_fail_closed_conversion_clause_persists_in_spec", + "tests/spec_conformance/test_cursor_hook_reqs.py::test_cursor_native_fail_closed_conversion_is_real_not_prose" + ] + }, + { + "conformance_class": "consumer", + "id": "req-tg-017", + "keyword": "MUST", + "section": "8.5.9", + "status": "active", + "test_count": 3, + "tests": [ + "tests/spec_conformance/test_cursor_hook_reqs.py::test_cursor_claude_import_overlap_rejection_clause_persists_in_spec", + "tests/spec_conformance/test_cursor_hook_reqs.py::test_cursor_claude_import_rejects_real_default_same_event_kind_content_overlap", + "tests/spec_conformance/test_cursor_hook_reqs.py::test_cursor_claude_overlap_predicate_is_kind_aware_not_event_only" + ] } ], "spec_version": "v0.1.1", "summary_by_class": { "consumer": { - "active": 92, + "active": 94, "skipped": 1, "unbound": 0, "xfail": 0 @@ -1584,5 +1610,5 @@ "xfail": 0 } }, - "total_requirements": 126 + "total_requirements": 128 } diff --git a/CONFORMANCE.md b/CONFORMANCE.md index 79add1ad2..0b42ac67a 100644 --- a/CONFORMANCE.md +++ b/CONFORMANCE.md @@ -33,7 +33,7 @@ APM claims the proposed deployed-prompt-audit capability (req-pl-019, req-pl-020 | Class | Active | Skipped | Xfail | Unbound | |-------|-------:|--------:|------:|--------:| | Producer | 12 | 0 | 0 | 0 | -| Consumer | 92 | 1 | 0 | 0 | +| Consumer | 94 | 1 | 0 | 0 | | Registry | 1 | 0 | 0 | 0 | | Governance | 20 | 0 | 0 | 0 | @@ -65,7 +65,7 @@ APM claims the proposed deployed-prompt-audit capability (req-pl-019, req-pl-020 | [req-lk-018](docs/src/content/docs/specs/openapm-v0.1.md#req-lk-018) | SHOULD | 5.5 | consumer | active | 1 | - | | [req-lk-019](docs/src/content/docs/specs/openapm-v0.1.md#req-lk-019) | MUST | 5.2 | consumer | active | 1 | - | | [req-lk-020](docs/src/content/docs/specs/openapm-v0.1.md#req-lk-020) | MUST | 5.2 | consumer | active | 3 | - | -| [req-lk-021](docs/src/content/docs/specs/openapm-v0.1.md#req-lk-021) | MUST | 5.2 | consumer | active | 2 | - | +| [req-lk-021](docs/src/content/docs/specs/openapm-v0.1.md#req-lk-021) | MUST | 5.2 | consumer | active | 3 | - | | [req-lk-022](docs/src/content/docs/specs/openapm-v0.1.md#req-lk-022) | MUST | 5.2 | consumer | active | 6 | - | | [req-mf-001](docs/src/content/docs/specs/openapm-v0.1.md#req-mf-001) | MUST | 4.1 | producer | active | 1 | - | | [req-mf-002](docs/src/content/docs/specs/openapm-v0.1.md#req-mf-002) | MUST | 4.1 | producer | active | 1 | - | @@ -167,6 +167,8 @@ APM claims the proposed deployed-prompt-audit capability (req-pl-019, req-pl-020 | [req-tg-013](docs/src/content/docs/specs/openapm-v0.1.md#req-tg-013) | MUST | 8.5.7 | consumer | active | 7 | - | | [req-tg-014](docs/src/content/docs/specs/openapm-v0.1.md#req-tg-014) | MUST | 8.5.8 | consumer | active | 1 | - | | [req-tg-015](docs/src/content/docs/specs/openapm-v0.1.md#req-tg-015) | MUST | 8.5.1 | consumer | active | 1 | - | +| [req-tg-016](docs/src/content/docs/specs/openapm-v0.1.md#req-tg-016) | MUST | 8.5.9 | consumer | active | 2 | - | +| [req-tg-017](docs/src/content/docs/specs/openapm-v0.1.md#req-tg-017) | MUST | 8.5.9 | consumer | active | 3 | - | ## Waivers diff --git a/docs/public/specs/manifests/openapm-v0.1.requirements.yml b/docs/public/specs/manifests/openapm-v0.1.requirements.yml index 080f7b463..5a7c99ed7 100644 --- a/docs/public/specs/manifests/openapm-v0.1.requirements.yml +++ b/docs/public/specs/manifests/openapm-v0.1.requirements.yml @@ -465,6 +465,16 @@ requirements: section: "8.5.1" conformance_class: consumer notes: "a conforming consumer providing the Codex-native agent conversion capability preserves a source-declared model or model_reasoning_effort string value in its native top-level placement, leaves either field absent when absent from the source, diagnoses a non-string source value by field name without including that value, and emits a diagnostic bounded on dropped-field count (at least one shown when any are dropped), per-field name length, and ASCII-sanitized field-name display (never the value) for any other dropped, non-capability-restriction frontmatter field" + - id: req-tg-016 + keyword: MUST + section: "8.5.9" + conformance_class: consumer + notes: "a Cursor-native hook installation implementation fails closed (no partial write) when converting a source-declared hook configuration whose event, top-level key, or handler field falls outside the accepted Cursor-native vocabulary, emitting an actionable diagnostic naming the unrecognized value(s)" + - id: req-tg-017 + keyword: MUST + section: "8.5.9" + conformance_class: consumer + notes: "a Cursor-native hook installation implementation detects, before writing, an observable overlap with an in-effect Claude-settings hook import for either install order, and rejects the write with an actionable diagnostic rather than merging, redirecting, or broadening accepted input, without altering the Claude import's own settings" - id: req-pr-006 keyword: MUST section: "8.1" diff --git a/docs/src/content/docs/producer/author-primitives/hooks-and-commands.md b/docs/src/content/docs/producer/author-primitives/hooks-and-commands.md index 7265b8167..f36d3c416 100644 --- a/docs/src/content/docs/producer/author-primitives/hooks-and-commands.md +++ b/docs/src/content/docs/producer/author-primitives/hooks-and-commands.md @@ -50,7 +50,7 @@ Claude (`PreToolUse`, `PostToolUse`) and Copilot (`preToolUse`, | `Stop`, `AgentStop`, `agentStop` | `agentStop` | `Stop` | | `UserPromptSubmit`, `userPromptSubmit`, `userPromptSubmitted` | `userPromptSubmitted` | `UserPromptSubmit` (native; the other two spellings are not renamed and will not fire) | -Event names absent from this table are preserved unchanged. Only an unmapped +For Copilot and Claude, event names absent from this table are preserved unchanged. Only an unmapped camelCase or PascalCase name that conflicts with the target convention emits an install warning. All-lowercase names such as `stop` pass through silently; `stop` is not a native Copilot or Claude event and will not fire. @@ -202,6 +202,84 @@ receives its current v1 standalone schema: `{ "version": "v1", "hooks": [{ "name", "trigger", "matcher", "action" }] }`. Kiro trigger names are PascalCase and command timeouts remain in seconds. +### Cursor native hooks and Claude import + +Cursor requires `{"version": 1, "hooks": {...}}` with flat handlers. +APM accepts native Cursor event names and these documented Claude aliases: + +| Claude source | Cursor native event | +|---------------|---------------------| +| `PreToolUse` | `preToolUse` | +| `PostToolUse` | `postToolUse` | +| `UserPromptSubmit` | `beforeSubmitPrompt` | +| `Stop` | `stop` | +| `SubagentStop` | `subagentStop` | +| `SessionStart` | `sessionStart` | +| `SessionEnd` | `sessionEnd` | +| `PreCompact` | `preCompact` | + +For example, a Claude `PreToolUse` group with matcher `Bash` becomes: + +```json +{ + "version": 1, + "hooks": { + "preToolUse": [ + {"type": "command", "command": ".cursor/hooks/my-package/hooks/check.sh", "matcher": "Shell", "timeout": 10} + ] + } +} +``` + +Native Cursor spellings keep native matcher values unchanged. For the Claude +`PreToolUse` and `PostToolUse` aliases, APM translates literal tool alternatives: +`Bash` becomes `Shell`; `Read`, `Grep`, `Task`, `WebFetch`, and `WebSearch` keep +their names. `Edit|Write` becomes `Write`; either one alone is rejected because +Cursor combines both operations. Missing matchers and the unrestricted `""`, +`*`, and `.*` forms remain unrestricted. + +APM rejects unknown events, other translated regexes, `Glob`, server-qualified +MCP matchers, and non-unrestricted matchers on other Claude aliases rather +than broadening their reach. It also rejects platform-specific commands, +unsupported handler fields/types, and invalid timeouts. Source documents may +contain `hooks`, optional `version: 1`, and a descriptive `description` string; +other source-level settings are rejected rather than silently discarded. Command and prompt +handlers retain native fields such as `failClosed`, `model`, and `loop_limit`. +Claude `Stop` and `SubagentStop` aliases get `loop_limit: null` to preserve the +documented unlimited import default rather than Cursor's native default of five. +Native Cursor hooks keep their own defaults. + +Cursor's **Include Third-Party Plugins, Skills, and Other Configs** setting is +on by default. All matching native and Claude-imported hooks run; native hooks +do not replace imported hooks. APM therefore refuses overlapping native/import +actions before deploying the package's primitives. Checks cover planned +Claude/Cursor sources and existing project, project-local, and user hook +configs. "Owner" here means the installing dependency's own declared +provenance marker for that hook entry (recorded in the sidecar described +below). Same-owner events -- an event already attributed to the package +about to install or retain the other side's configuration -- are treated +conservatively as overlapping even when matchers differ; APM does not try +to prove arbitrary regexes disjoint. + +Choose one hook deployment route per dependency. To reuse existing Claude +hooks, select `claude` for that dependency and verify third-party imports are +enabled in Cursor. To use native-only events such as `beforeReadFile`, author +Cursor-native hooks and select `cursor`. APM neither changes import settings +nor writes Claude configuration for a Cursor-only dependency. Different +scripts can still have overlapping behavior; test them in the actual harness. + +Reinstall migrates matching APM-owned legacy Cursor entries and keeps unrelated +user hooks. Switching a dependency to one route can retire its old project +route; it does not remove user hooks or matching hooks in another scope. +Invalid or unsupported user configuration is rejected without being +overwritten. Target-specific files retain the existing filename-routing rules; +this repair does not add per-file target declarations or dry-run hook previews. + +These mappings follow the [Cursor hook reference](https://cursor.com/docs/hooks) +and [third-party hook reference](https://cursor.com/docs/reference/third-party-hooks). +They are configuration compatibility, not a guarantee of identical execution +semantics. Cloud Cursor agents support command hooks only. + Copilot hook files are namespaced with the source package name to avoid collisions across installed deps; bundled scripts land alongside under `.github/hooks/scripts//`. @@ -277,10 +355,15 @@ agent a procedure" fits a skill -- and reaches every harness. ## Pitfalls - **Hook event names.** Use the documented - [session lifecycle aliases](#session-lifecycle-event-aliases). Unknown names - are preserved. An install warning appears only when an unmapped name starts + [session lifecycle aliases](#session-lifecycle-event-aliases) for Copilot + and Claude. Their unknown names are preserved. An install warning appears + only when an unmapped name starts with a capital letter or starts lowercase and contains a later capital - letter, and that casing conflicts with the target convention. + letter, and that casing conflicts with the target convention. Cursor + instead rejects unsupported events, fields and restrictions; see + [Cursor native hooks](#cursor-native-hooks-and-claude-import). Diagnostics + name unsupported source-level, matcher-group and handler fields and identify + whitespace in literal matcher alternatives without silently stripping it. - **Cursor command frontmatter loss.** Cursor reuses the Claude command transformer today, so any prompt-only metadata is dropped with a diagnostic. Keep Cursor commands to the preserved key set. @@ -308,6 +391,7 @@ agent a procedure" fits a skill -- and reaches every harness. a Claude+OpenCode package and assume hooks reach both -- they do not. The install log notes the skip. -Once your hooks and commands are in place, run `apm install --dry-run` -to preview what each target will receive, then `apm pack` to bundle. +Once your hooks and commands are in place, inspect a real installation's +native configuration, then run `apm pack` to bundle. +`apm install --dry-run` does not render hook previews. See [Compile](../../compile/) and [Pack a bundle](../../pack-a-bundle/). diff --git a/docs/src/content/docs/reference/cli/audit.md b/docs/src/content/docs/reference/cli/audit.md index 55124ecc4..15b25dd8b 100644 --- a/docs/src/content/docs/reference/cli/audit.md +++ b/docs/src/content/docs/reference/cli/audit.md @@ -201,7 +201,7 @@ not every file in a skill bundle. |---|---|---| | Copilot | `.github/instructions/*.instructions.md`, `prompts/*.prompt.md`, `agents/*.agent.md`, `.github/copilot-instructions.md`; shared skills | `.github/hooks/*.json`; commands are non-prompt | | Claude | `.claude/rules/*.md`, `agents/*.md`, `commands/*.md`, `skills/**/SKILL.md` | `.claude/hooks/*.json`, `.claude/settings.json`; nested `prompt`/`agent` handlers' `prompt` field | -| Cursor | `.cursor/rules/*.mdc`, `agents/*.md`, `commands/*.md`; shared skills | `.cursor/hooks/*.json`, `.cursor/hooks.json`; flat or APM-preserved nested command handlers | +| Cursor | `.cursor/rules/*.mdc`, `agents/*.md`, `commands/*.md`; shared skills | `.cursor/hooks/*.json`, `.cursor/hooks.json`; flat command handlers and prompt handlers' `prompt` field; nested handlers are unsupported | | Kiro | `.kiro/steering/*.md`, `agents/*.md`, `skills/**/SKILL.md` | `.kiro/hooks/*.json`; v1 `hooks[].action.prompt` for agent actions | | Gemini | `.gemini/commands/*.toml`: `prompt`; shared skills | `.gemini/hooks/*.json`, `.gemini/settings.json`; nested command handlers | | Codex | `.codex/agents/*.toml`: `developer_instructions`; shared skills | `.codex/hooks.json`; nested command handlers | diff --git a/docs/src/content/docs/reference/common-errors.md b/docs/src/content/docs/reference/common-errors.md index 03097ad38..ca37d8383 100644 --- a/docs/src/content/docs/reference/common-errors.md +++ b/docs/src/content/docs/reference/common-errors.md @@ -36,6 +36,11 @@ APM v0.21.0+ always writes `"version": 1` to `.cursor/hooks.json` on a fresh install. Existing files that already contain a `"version"` key are left untouched. +If installation instead rejects unsupported Cursor events or reports overlap +with Claude import, follow the +[native hook compatibility and one-route guidance](../../producer/author-primitives/hooks-and-commands/#cursor-native-hooks-and-claude-import). +APM does not overwrite invalid user hooks or change Cursor's import settings. + --- ### Agent Plugin does not load in Copilot diff --git a/docs/src/content/docs/specs/openapm-v0.1.md b/docs/src/content/docs/specs/openapm-v0.1.md index d8c2ecd71..9a8a5429f 100644 --- a/docs/src/content/docs/specs/openapm-v0.1.md +++ b/docs/src/content/docs/specs/openapm-v0.1.md @@ -136,7 +136,7 @@ between the companion corpus and the implementation. ### 1.3 Document conventions -- OpenAPM v0.1 carries **126 normative statements (121 MUST, 5 SHOULD)** indexed in +- OpenAPM v0.1 carries **128 normative statements (123 MUST, 5 SHOULD)** indexed in [Appendix C](#appendix-c-index-of-normative-statements). - All on-disk files defined by this specification are **YAML 1.2** parsed under the safe subset defined in @@ -2977,6 +2977,104 @@ manifest restriction, it MUST serialize only the supported subset using target identifiers whose replay selects the same runtimes; it MUST NOT persist an unsupported member or remap one to a different supported runtime. +#### 8.5.9 Cursor-native hook installation: fail-closed validation and Claude-import coexistence + + +**[req-tg-016]** A conforming consumer implementation that provides Cursor-native +hook installation MUST fail closed when converting a source-declared hook +configuration into that Cursor-native format: if any source-declared event, +top-level configuration key, or handler field falls outside the vocabulary of +events, top-level keys, and handler fields the implementation accepts for +Cursor-native hooks, the implementation MUST NOT write the Cursor-native hook +artifact (zero bytes, no partial file) and MUST emit an actionable diagnostic +naming the unrecognized value(s). Implementations SHOULD document or +programmatically expose the set of accepted source-format identifiers so that +conformance claims are independently verifiable. + + +**[req-tg-017]** A conforming consumer implementation that provides Cursor-native +hook installation, when the project or user scope also has a Claude-settings +hook import in effect for the same scope, MUST detect before writing whether +installing or retaining the Cursor-native hook configuration would create or +extend an observable overlap with that Claude-imported hook configuration, for +both install orders (Claude-import-first and Cursor-native-first). On a +detected overlap the implementation MUST reject the write with an actionable +diagnostic rather than merge, redirect, or broaden either side's accepted +input, and MUST NOT alter the Claude import's own settings. + +(a) For the purposes +of this requirement, "observable overlap" is a non-empty intersection, after +alias normalization, between the set of (event identifier, handler kind, +handler content) tuples claimed by the Cursor-native configuration and those +claimed by the in-effect Claude-settings import. + +(b) Alias normalization maps +each side's source-declared event identifier to the vocabulary of event +identifiers this implementation accepts for Cursor-native hooks (see +[req-tg-016](#req-tg-016)); req-tg-016 leaves that accepted vocabulary +implementation-defined, so this clause does not assert that req-tg-016 +itself fixes a concrete alias-to-identifier mapping. + +(c) Two entries for the +same event identifier but a different handler kind or different handler +content are not an overlap, since the same event MAY legitimately carry +distinct, independently-triggered handlers on each side; handler-content +comparison is a literal-string comparison performed after this +implementation's existing deployed-package-root substitutions, and it does +not trim whitespace or infer executable- or matcher-level equivalence +between differently-formatted handler commands. + +(d) Declared source package +ownership is the exception to that content comparison: when the +installing implementation's own internal provenance marker (recorded for +later reinstall/removal cleanup and never written into the Claude side) +already attributes an entry on the Cursor-native side and an entry on the +Claude-imported side to the SAME source package for the same event, that +pairing counts as an overlap regardless of handler kind or handler +content, because that package would otherwise claim two +independently-activated copies of its own hook for the same event. + +(e) This +provenance check and the rest of the tuple comparison both apply +identically for both install orders; neither is evaluated only for +Cursor-native-first or only for Claude-import-first. + +(f) Absence of a +provenance marker on an entry (for example, because the entry predates +this implementation's tracking, or an externally edited settings file +dropped the sidecar) is not itself malformed configuration and is not +redefined as an overlap; without an established same-package attribution +on both sides, the event falls back to the ordinary (event identifier, +handler kind, handler content) tuple comparison like any other entry. +This implementation's existing fail-closed handling for an actually +unreadable or structurally invalid configuration document is unrelated to, +and not triggered by, ordinary marker absence. This is an APM +consumer-side coexistence policy choice, not a bare event-identifier-only +comparison. + +> **Editorial note.** The event vocabulary, the Claude-to-Cursor event +> aliases, and the two-key (`version`, `hooks`) top-level shape this +> implementation validates are drawn from Cursor's own hook reference and +> third-party-hook reference as published at +> `https://cursor.com/docs/hooks` and +> `https://cursor.com/docs/reference/third-party-hooks` (fetched 2026-10-03). +> As with [req-tg-009](#req-tg-009)'s target-native capability encodings, these concrete +> vendor-specific vocabularies are intentionally kept out of the normative +> text above; a future harness implementing the same Cursor-native capability +> may accept a different, equally fail-closed vocabulary as the vendor +> surface evolves. The vendor reference also documents a `prompt` handler +> kind alongside `command`; this note does not assert that the accepted +> vocabulary covers every hook feature Cursor documents, only that whatever +> vocabulary an implementation does accept is enforced fail-closed. The +> rejection in [req-tg-017](#req-tg-017) is an APM consumer-side safety policy, not a +> reflection of Cursor's own documented default: the vendor's third-party-hook +> reference states that when hooks exist in multiple locations "All matching +> hooks from every source run" (merge, not reject). Likewise, the unknown-key +> rejection in [req-tg-016](#req-tg-016) is this implementation's own conservative +> conversion policy; the cited vendor example shows only `version` and +> `hooks` as top-level keys but does not itself document the top-level shape +> as closed to extension. + ### 8.6 Per-target primitive support (informational) The matrix of which primitive types each target supports is @@ -2995,7 +3093,9 @@ without a spec revision. The current matrix is in the companion [req-tg-010](#req-tg-010), [req-tg-011](#req-tg-011), [req-tg-012](#req-tg-012), [req-tg-013](#req-tg-013), [req-tg-014](#req-tg-014), [req-tg-015](#req-tg-015), - [req-pr-006](#req-pr-006), [req-pr-007](#req-pr-007). + [req-pr-006](#req-pr-006), + [req-pr-007](#req-pr-007), [req-tg-016](#req-tg-016), + [req-tg-017](#req-tg-017). --- @@ -3379,6 +3479,7 @@ every stored hash, foreclosing algorithm-ambiguity attacks. | 19| Executable deployment in non-interactive contexts | [req-sc-014](#req-sc-014) | Consumer-default | | 20| Source-only or symlinked package content materialization | [req-sc-015](#req-sc-015) | Consumer-default | | 21| Native plugin namespace collision or ownership-ledger loss | [req-tg-013](#req-tg-013) | Consumer-default | +| 22| Silent hook passthrough or double-activation via native-format conversion | [req-tg-016](#req-tg-016), [req-tg-017](#req-tg-017) | Consumer-default | For Codex conversion metadata diagnostics, [req-tg-015](#req-tg-015) separately constrains field-name display and excludes dropped values. @@ -3642,6 +3743,7 @@ conformance statement identifying: [req-tg-010](#req-tg-010), [req-tg-011](#req-tg-011), [req-tg-012](#req-tg-012), [req-tg-013](#req-tg-013), [req-tg-014](#req-tg-014), [req-tg-015](#req-tg-015), +[req-tg-016](#req-tg-016), [req-tg-017](#req-tg-017), [req-sc-001](#req-sc-001), [req-sc-002](#req-sc-002), [req-sc-003](#req-sc-003), [req-sc-004](#req-sc-004), [req-sc-005](#req-sc-005), @@ -4105,6 +4207,8 @@ renumbering of conformance classes. | [req-tg-013](#req-tg-013) | MUST | 8.5.7 | consumer | | [req-tg-014](#req-tg-014) | MUST | 8.5.8 | consumer | | [req-tg-015](#req-tg-015) | MUST | 8.5.1 | consumer | +| [req-tg-016](#req-tg-016) | MUST | 8.5.9 | consumer | +| [req-tg-017](#req-tg-017) | MUST | 8.5.9 | consumer | | [req-sc-001](#req-sc-001) | MUST | 10.4 | consumer | | [req-sc-002](#req-sc-002) | MUST | 10.9 | consumer | | [req-sc-003](#req-sc-003) | MUST | 10.3 | consumer | @@ -4124,7 +4228,7 @@ renumbering of conformance classes. | [req-cf-001](#req-cf-001) | MUST | 12.5 | consumer | | [req-cf-002](#req-cf-002) | MUST | 12.3 | consumer | -**Total normative statements: 126** (121 MUST, 5 SHOULD). +**Total normative statements: 128** (123 MUST, 5 SHOULD). --- @@ -4177,6 +4281,7 @@ renumbering of conformance classes. | 0.1.41 | 2026-09-09 | Alias containment and lock-replay contract for PR #2901. Added [req-mf-025] (Section 4.3.2, consumer MUST), the optional lock-entry `alias` field, and conformance coverage. Under Section 9.2 this is an additive optional field and a defensive definition of previously unspecified alias behavior, not behavior-neutral errata: unsafe or reserved aliases can newly fail; valid dotted aliases remain accepted; surrounding whitespace is canonicalized; recorded aliases determine replay placement; absent aliases retain the unaliased layout. Source identity and permitted local source paths are unchanged. Older readers preserving the unknown field do not thereby implement placement support. Selects distinct 0.1.41 schema publication identities without changing published v0.1 URLs or bytes; Section 9.3 remains pending (see Appendix A). Sections 1.3, 4.9, 5.2, 10.7, 10.11, 11.3.2, and Appendix C updated. Statement count: 122 -> 123 (118 MUST, 5 SHOULD). | | 0.1.42 (proposed) | 2026-09-30 | Optional deployed-prompt audit capability for PR #2962. Added conditional governance [req-pl-019] and [req-pl-020] in Section 6.8.1, both enumerations, Appendix C, requirements manifest and behavioral conformance coverage. Under Section 9.2 this is a new opt-in conformance capability, not behavior-neutral errata or a reinterpretation of an existing obligation: implementations not claiming it acquire no new required feature. APM claims it; newly discovered prompt findings and incomplete native coverage can newly fail default and CI audits, command-only content remains non-failing, and protected remediation is refused before writes. No schema, lockfile version, existing mandatory feature, drift-policy or ownership rule changes. Section 9.3 reviewer approvals and public comment period remain pending; this proposal is not evidence of adoption. Statement count: 123 -> 125 (120 MUST, 5 SHOULD). | | 0.1.43 (proposed) | 2026-10-03 | Codex-native agent model preservation and bounded dropped-metadata diagnostic for PR #3150 (closes #3126). Added [req-tg-015] (Section 8.5.1, consumer MUST): a conforming consumer providing the Codex-native agent conversion capability MUST preserve a source-declared `model` or `model_reasoning_effort` string value in its native top-level placement, MUST leave either field absent when absent from the source, MUST diagnose a non-string value by field name without including that value, and MUST emit a diagnostic bounded on both dropped-field count and per-field name length (never the value) for any other dropped, non-capability-restriction frontmatter field. Under Section 9.2 this is a new opt-in conformance capability scoped to consumers providing the Codex-native agent conversion capability, not behavior-neutral errata: it does not require or imply preservation of any other `config.toml`-native key and does not define behavior for any other conversion target. Implementations not providing Codex-native agent conversion acquire no new required feature. No schema, lockfile version, existing mandatory feature, drift-policy or ownership rule changes. Section 9.3 reviewer approvals and public comment period remain pending; this proposal is not evidence of adoption. Section 8.7, Section 11.3.2, and Appendix C updated. Statement count: 125 -> 126 (121 MUST, 5 SHOULD). | +| 0.1.44 (proposed) | 2026-10-03 | PR #3149 adds two consumer MUSTs in Section 8.5.9: [req-tg-016](#req-tg-016) rejects unsupported Cursor hook vocabulary; [req-tg-017](#req-tg-017) detects overlap with Claude imports before either install order writes, preserving import settings. These cite the accepted Cursor capability, not universal translation or a new mandatory feature (Section 9.2). Section 8.7, Section 11.3.2 and Appendix C are updated. Section 9.3 reviewer approvals and public comment remain pending; this proposal is not adopted. Statement count: 126 -> 128 (123 MUST, 5 SHOULD). | Errata (none at publication). diff --git a/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md b/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md index 9d2bf3e00..832575ee4 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md +++ b/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md @@ -184,6 +184,18 @@ hook action under `.kiro/hooks/`. For Codex, APM wraps flat command entries in hook groups containing a nested `hooks` array in `.codex/hooks.json`. +For Cursor, APM writes native v1 flat command or prompt handlers using only +verified aliases. Unsupported fields, restrictions and overlapping Claude +imports fail before writes; diagnostics name unsupported source-level, +matcher-group and handler fields and distinguish whitespace in literal matcher +alternatives. Choose one hook route per dependency: +native `cursor`, or `claude` with Cursor's existing import. APM never changes +import settings. Reinstall and target retirement preserve unrelated user hooks. +See the +[Cursor native hooks guide](../../../../../docs/src/content/docs/producer/author-primitives/hooks-and-commands.md#cursor-native-hooks-and-claude-import) +for supported mappings and limitations. Test execution in each harness; +configuration compatibility does not guarantee equivalent behavior. + ### Session lifecycle event aliases @@ -194,7 +206,7 @@ nested `hooks` array in `.codex/hooks.json`. | `Stop`, `AgentStop`, `agentStop` | `agentStop` | `Stop` | | `UserPromptSubmit`, `userPromptSubmit`, `userPromptSubmitted` | `userPromptSubmitted` | `UserPromptSubmit` (native; the other two spellings are not renamed and will not fire) | -Event names absent from this table are preserved unchanged. Only an unmapped +For Copilot and Claude, event names absent from this table are preserved unchanged. Only an unmapped camelCase or PascalCase name that conflicts with the target convention emits an install warning. All-lowercase names such as `stop` pass through silently; `stop` is not a native Copilot or Claude event and will not fire. diff --git a/scripts/architecture_linter/checks/mutation_hook_contract.py b/scripts/architecture_linter/checks/mutation_hook_contract.py index 808295e61..3024aba2b 100644 --- a/scripts/architecture_linter/checks/mutation_hook_contract.py +++ b/scripts/architecture_linter/checks/mutation_hook_contract.py @@ -44,6 +44,7 @@ from scripts.architecture_linter.groups.common import ( EXEMPT_MARKER, checked_facts, + forbid_text, line_pattern_violations, violation, ) @@ -79,6 +80,9 @@ _INSTALL_DRIFT = "src/apm_cli/install/drift.py" +_DEPLOYABLE_SOURCE_PLAN = "src/apm_cli/install/deployable_source_plan.py" + + _HOOK_FILE_ROUTING_TARGETS: tuple[str, ...] = (_HOOK_INTEGRATOR, _KIRO_HOOK_INTEGRATOR) @@ -340,7 +344,11 @@ def _check_neutral_hook_contract(provider: FactsProvider) -> Iterable[Violation] keys route through ``hook_contract``, per-file routing is not gated by dependency targets, merge-hook config writes stay owned by ``HookIntegrator``, ownership markers route through ``hook_ownership``, - and shared drift projection routes through ``hook_ownership``. + shared drift projection routes through ``hook_ownership``, and Cursor + import preflight stays unconditional at every native-write call site + (#3129: a cached "already preflighted" flag on the plan object must + never gate the re-check, because a plan can be reused across projects + or after a Claude import file is added mid-install). """ rule_id = "mutation_writes.neutral_hook_contract" return ( @@ -348,6 +356,8 @@ def _check_neutral_hook_contract(provider: FactsProvider) -> Iterable[Violation] *_nhc_rewrite_scope(provider, rule_id), *_nhc_claude_project_dir(provider, rule_id), *_nhc_event_map(provider, rule_id), + *_nhc_cursor_edge(provider, rule_id), + *_nhc_cursor_preflight_unconditional(provider, rule_id), *_nhc_contract_vocabulary(provider, rule_id), *_nhc_command_keys(provider, rule_id), *_nhc_file_routing(provider, rule_id), @@ -497,6 +507,135 @@ def _nhc_contract_vocabulary(provider: FactsProvider, rule_id: str) -> tuple[Vio ) +def _nhc_cursor_edge(provider: FactsProvider, rule_id: str) -> tuple[Violation, ...]: + """Keep Cursor rendering and import preflight on the canonical native edge.""" + renderer = "src/apm_cli/integration/hook_native_formats.py" + preflight = "src/apm_cli/integration/hook_cursor_preflight.py" + services = "src/apm_cli/install/services.py" + facts, failures = _read_required( + provider, rule_id, (renderer, preflight, _HOOK_INTEGRATOR, services) + ) + findings: list[Violation] = list(failures) + if not failures: + for path, required in ( + (_HOOK_INTEGRATOR, "entries = _to_cursor_hook_entries("), + (_HOOK_INTEGRATOR, "preflight_cursor_hooks("), + (preflight, "_to_cursor_hook_entries("), + (preflight, "validate_cursor_config(candidate)"), + (renderer, "_check_cursor_field_types(raw)"), + (renderer, "_check_cursor_field_types(entry)"), + (services, '"preflight_hooks_for_targets"'), + (services, "preflight_hooks("), + ): + findings.extend( + _require( + _has_fixed(facts[path], required), + rule_id, + path, + "Cursor hooks must use canonical rendering and import preflight before writes", + ) + ) + findings.extend( + _duplicate_scan( + provider, + rule_id=rule_id, + paths=_python_paths(provider, under=_SRC, exclude=(renderer,)), + pattern=r"^(CURSOR_NATIVE_EVENTS\s*[:=]|def _to_cursor_hook_entries\()", + message="Cursor native hook contract must have one renderer owner", + exempt=False, + ) + ) + return tuple(findings) + + +def _nhc_cursor_preflight_unconditional( + provider: FactsProvider, rule_id: str +) -> tuple[Violation, ...]: + """Cursor/Claude import preflight must never be gated by a cached flag. + + Two independent sub-checks close the gap a purely lexical "preflight + call is present somewhere in the file" check (see ``_nhc_cursor_edge``) + cannot see: + + * A token ban on ``cursor_preflight_done`` across the owner files, so a + plan-level cache field can never reintroduce the reused-plan bypass + fixed for #3129 under that name. + * A structural, per-call-site check that the native-write preflight + call at the per-target write boundary in ``_integrate_merged_hooks`` + survives as a direct (unconditional) statement of its owning + ``if config.target_key in {...}:`` block -- not removed, and not + re-wrapped in a *second*, nested conditional (any cache predicate, + regardless of its name, would have to be such a nested ``if``). A + file-wide substring check cannot tell a removed/re-gated call at one + write boundary from an unrelated surviving call at another (for + example the separate upfront ``preflight_hooks_for_targets`` call), + because deleting or re-gating one occurrence still leaves the + ``preflight_cursor_hooks(`` substring present elsewhere in the file. + """ + findings: list[Violation] = [] + for path in (_HOOK_INTEGRATOR, _DEPLOYABLE_SOURCE_PLAN): + findings.extend( + forbid_text( + provider, + rule_id=rule_id, + path=path, + needles=("cursor_preflight_done",), + message=( + "Cursor import preflight must stay unconditional; " + "a cached cursor_preflight_done flag must never gate it" + ), + ) + ) + findings.extend(_nhc_cursor_preflight_call_site(provider, rule_id)) + return tuple(findings) + + +def _nhc_cursor_preflight_call_site(provider: FactsProvider, rule_id: str) -> tuple[Violation, ...]: + """Guard the exact per-write preflight call site structurally. + + Looks up ``HookIntegrator._integrate_merged_hooks``, finds its direct + ``if config.target_key in {'cursor', 'claude'}:`` statement (the only + legitimate conditional around the call -- preflight only applies to + those two targets), and requires the ``preflight_cursor_hooks(...)`` + call to be a direct statement of that ``if`` body. A removed call, or + one re-wrapped in any further nested conditional (a cache gate under + any name), fails this check even though the ``preflight_cursor_hooks(`` + substring would still appear elsewhere in the file. + """ + message = ( + "HookIntegrator._integrate_merged_hooks must call " + "preflight_cursor_hooks(...) as a direct, unconditional statement " + "of its 'if config.target_key in {cursor, claude}' block -- not " + "removed and not re-wrapped in a further (cache) conditional" + ) + index = provider.tree_index(_HOOK_INTEGRATOR) + if index is None: + return (violation(rule_id, _HOOK_INTEGRATOR, message),) + function = index.function("HookIntegrator._integrate_merged_hooks") + if function is None: + return (violation(rule_id, _HOOK_INTEGRATOR, message),) + target_key_if = next( + ( + stmt + for stmt in function.body + if isinstance(stmt, ast.If) + and ast.unparse(stmt.test) == "config.target_key in {'cursor', 'claude'}" + ), + None, + ) + if target_key_if is None: + return (violation(rule_id, _HOOK_INTEGRATOR, message),) + has_direct_call = any( + isinstance(stmt, ast.Expr) + and isinstance(stmt.value, ast.Call) + and ast.unparse(stmt.value.func) == "preflight_cursor_hooks" + for stmt in target_key_if.body + ) + if has_direct_call: + return () + return (violation(rule_id, _HOOK_INTEGRATOR, message),) + + def _nhc_command_keys(provider: FactsProvider, rule_id: str) -> tuple[Violation, ...]: """Neutral hook source grammar must route through ``hook_contract.py``.""" owner_count = sum( diff --git a/src/apm_cli/install/services.py b/src/apm_cli/install/services.py index 606003241..964cbe4ab 100644 --- a/src/apm_cli/install/services.py +++ b/src/apm_cli/install/services.py @@ -398,14 +398,7 @@ def _reconcile_excluded_targets() -> None: from apm_cli.install.target_warnings import warn_unsupported_primitives - warn_unsupported_primitives( - package_info, - package_name, - targets, - ctx, - diagnostics, - logger, - ) + warn_unsupported_primitives(package_info, package_name, targets, ctx, diagnostics, logger) def _log_integration(msg): if logger: @@ -423,6 +416,16 @@ def _log_integration(msg): "skills": integrators.skill, } + preflight_hooks = getattr(integrators.hook, "preflight_hooks_for_targets", None) + if _hooks_approved and callable(preflight_hooks): + preflight_hooks( + package_info, + project_root, + source_plan, + user_scope=scope is InstallScope.USER, + retiring_targets=frozenset(target.name for target in target_selection.excluded_targets), + ) + # Validate every converted instruction target before any primitive kind can # write. A rejected instruction must not leave prompts, agents, commands, # or identity-target instructions from the same package active. @@ -500,6 +503,11 @@ def _log_integration(msg): _call_kwargs["user_scope"] = scope is InstallScope.USER _call_kwargs["dep_targets_active"] = dep_targets_active _call_kwargs["allowed_targets"] = allowed_dep_targets + # Mirror retiring_targets (#3129) so a fallback preflight isn't + # misflagged as an import-coexistence conflict on retirement. + _call_kwargs["retiring_targets"] = frozenset( + t.name for t in target_selection.excluded_targets + ) # Canvas integration: always pass is_first_party. Approval # is enforced by the gate above (canvas already skipped if # not approved and not is_first_party), so here we always diff --git a/src/apm_cli/integration/hook_cursor_preflight.py b/src/apm_cli/integration/hook_cursor_preflight.py new file mode 100644 index 000000000..b5f708686 --- /dev/null +++ b/src/apm_cli/integration/hook_cursor_preflight.py @@ -0,0 +1,249 @@ +"""Read-only Cursor native validation and Claude-import coexistence checks.""" + +from __future__ import annotations + +import copy +import json +from pathlib import Path +from typing import TYPE_CHECKING, Any + +from apm_cli.hook_contract import HookContractError, hook_handlers +from apm_cli.integration.hook_native_formats import ( + CURSOR_NATIVE_EVENTS, + _to_cursor_hook_entries, + validate_cursor_config, +) +from apm_cli.integration.hook_ownership import reinject_apm_source_from_sidecar +from apm_cli.integration.hook_source_selection import HookSourceSelection +from apm_cli.utils.diagnostics import printable_ascii_text +from apm_cli.utils.path_security import ensure_path_within, has_symlink_component + +if TYPE_CHECKING: + from apm_cli.integration.hook_integrator import HookIntegrator + from apm_cli.models.apm_package import PackageInfo + + +def _read_config(path: Path, root: Path) -> dict[str, Any]: + """Read existing config without following links or replacing malformed content.""" + ensure_path_within(path, root) + if has_symlink_component(root, path): + raise HookContractError("hook configuration must not be a symlink") + if not path.exists(): + return {} + try: + data = json.loads(path.read_text(encoding="utf-8")) + except (OSError, ValueError) as exc: + raise HookContractError( + f"cannot read hook configuration {printable_ascii_text(path.name)}" + ) from exc + if not isinstance(data, dict): + raise HookContractError("hook configuration must be a JSON object") + return data + + +def _existing_hooks(path: Path, root: Path) -> dict[str, Any]: + data = _read_config(path, root) + sidecar = _read_config(path.with_name("apm-hooks.json"), root) + hooks = data.get("hooks", {}) + if not isinstance(hooks, dict) or any(not isinstance(v, list) for v in hooks.values()): + raise HookContractError("existing hook configuration requires event arrays") + if sidecar: + reinject_apm_source_from_sidecar(hooks, sidecar) + return data + + +def _without_retiring_owner(document: dict[str, Any], owners: set[str]) -> dict[str, Any]: + """Project the existing ownership decision without touching user entries.""" + return { + "hooks": { + event: [ + entry + for entry in entries + if not isinstance(entry, dict) + or not isinstance(entry.get("_apm_source"), str) + or entry["_apm_source"] not in owners + ] + for event, entries in document.get("hooks", {}).items() + } + } + + +def _action_keys( + document: dict[str, Any], + event_map: dict[str, str], + roots: tuple[str, ...], + owners: set[str], + *, + owned: bool = False, +) -> set[tuple[str, str, str]]: + """Compare declared actions, not matcher semantics or executable contents.""" + result: set[tuple[str, str, str]] = set() + for binding in hook_handlers(document): + event = event_map.get(binding.event) + if event is None: + continue + value = binding.value + source = value.get("_apm_source") + if owned or (isinstance(source, str) and source in owners): + result.add((event, "owner", "current-package")) + kind = value.get("type", "command") + content = value.get("prompt" if kind == "prompt" else "command") + if not isinstance(content, str): + continue + for root in roots: + content = content.replace(root + "/", "$APM_PACKAGE/") + result.add((event, str(kind), content)) + return result + + +def preflight_cursor_hooks( + integrator: HookIntegrator, + package_info: PackageInfo, + project_root: Path, + selection: HookSourceSelection, + event_maps: dict[str, dict[str, str]], + *, + user_scope: bool = False, + retiring_targets: frozenset[str] = frozenset(), +) -> None: + """Reject unsupported Cursor hooks or potential double activation before writes. + + Import settings are intentionally not read or changed: they are user-specific + and can change after installation. A shared action must have one deployment + route, not two configurations that are safe only while imports are disabled. + """ + from apm_cli.integration.targets import KNOWN_TARGETS + + cursor_root = project_root / KNOWN_TARGETS["cursor"].root_dir + cursor_files = selection.descriptors_for("cursor") + claude_files = selection.descriptors_for("claude") + if not cursor_files and not claude_files: + return + home = Path.home() + native_paths = [(cursor_root / "hooks.json", project_root)] + if home != project_root: + native_paths.append((home / KNOWN_TARGETS["cursor"].root_dir / "hooks.json", home)) + if not cursor_files and not any(path.exists() for path, _ in native_paths): + return + package_name = integrator._get_package_name(package_info, project_root) + source, legacy = integrator._get_hook_source_markers(package_info, project_root, package_name) + owners = {source, *legacy} + roots = tuple( + sorted( + { + f"{base}/{name}/hooks/{package_name}" + for base in (project_root.as_posix(), home.as_posix()) + for name in (".claude", ".cursor") + } + | {f"{name}/hooks/{package_name}" for name in (".claude", ".cursor")}, + key=len, + reverse=True, + ) + ) + native_map = {event: event for event in CURSOR_NATIVE_EVENTS} + cursor_map = event_maps["cursor"] + claude_map = { + **cursor_map, + **{ + alias: cursor_map[native] + for alias, native in event_maps["claude"].items() + if native in cursor_map + }, + } + + def source_actions(files: list[Path], *, cursor: bool) -> set[tuple[str, str, str]]: + actions: set[tuple[str, str, str]] = set() + for path in files: + document = integrator._parse_hook_json(path) + if document is None or not isinstance(document.get("hooks"), dict): + raise HookContractError(f"invalid hook source {printable_ascii_text(path.name)}") + rewritten, _ = integrator._rewrite_hooks_data( + document, + package_info.install_path, + package_name, + "cursor", + hook_file_dir=path.parent, + deploy_root=integrator._deploy_root_for_hook_rewrite(project_root, user_scope), + ) + if cursor: + if document.keys() - {"version", "hooks", "description"}: + raise HookContractError( + "unsupported Cursor source fields " + f"{sorted(document.keys() - {'version', 'hooks', 'description'})!r}; " + "no settings were discarded" + ) + if "description" in document and not isinstance(document["description"], str): + raise HookContractError("Cursor source description must be a string") + if "version" in document and ( + type(document["version"]) is not int or document["version"] != 1 + ): + raise HookContractError("Cursor source requires version 1 when specified") + for event, entries in rewritten["hooks"].items(): + _to_cursor_hook_entries( + entries, cursor_map.get(event, event), foreign=event in cursor_map + ) + actions.update( + _action_keys( + rewritten, + {**native_map, **cursor_map} if cursor else claude_map, + roots, + owners, + owned=True, + ) + ) + return actions + + try: + cursor_actions = source_actions(cursor_files, cursor=True) + claude_actions = source_actions(claude_files, cursor=False) + if cursor_files: + existing = _existing_hooks(cursor_root / "hooks.json", project_root) + candidate = copy.deepcopy(existing) + candidate.setdefault("version", 1) + candidate.update(_without_retiring_owner(candidate, owners)) + candidate["hooks"] = { + event: entries for event, entries in candidate["hooks"].items() if entries + } + validate_cursor_config(candidate) + imports = [ + (project_root / ".claude/settings.local.json", project_root), + (project_root / ".claude/settings.json", project_root), + (home / ".claude/settings.json", home), + ] + for path, root in dict.fromkeys(imports): + imported = _existing_hooks(path, root) + if "claude" in retiring_targets and path == project_root / ".claude/settings.json": + imported = _without_retiring_owner(imported, owners) + claude_actions.update( + _action_keys( + {"hooks": imported.get("hooks", {})}, + cursor_map, + roots, + owners, + ) + ) + if claude_files: + for path, root in native_paths: + existing = _existing_hooks(path, root) + if "cursor" in retiring_targets and path == cursor_root / "hooks.json": + existing = _without_retiring_owner(existing, owners) + cursor_actions.update( + _action_keys( + {"hooks": existing.get("hooks", {})}, + native_map, + roots, + owners, + ) + ) + if cursor_actions & claude_actions: + raise HookContractError( + "Cursor native hooks overlap Claude import and may run twice. " + "Select one hook target per dependency; existing Claude hooks can use " + "Cursor's third-party import instead. No import setting was changed" + ) + except HookContractError as exc: + raise HookContractError( + f"Cannot install hooks for {printable_ascii_text(package_name)}: " + f"{printable_ascii_text(str(exc))}. " + "Use supported Cursor-native hooks or a single Claude-import route, then reinstall." + ) from exc diff --git a/src/apm_cli/integration/hook_integrator.py b/src/apm_cli/integration/hook_integrator.py index c1606db17..13a1462a5 100644 --- a/src/apm_cli/integration/hook_integrator.py +++ b/src/apm_cli/integration/hook_integrator.py @@ -45,11 +45,13 @@ unresolved_plugin_root_references, ) from apm_cli.integration.hook_command_warnings import warn_unresolved_plugin_root +from apm_cli.integration.hook_cursor_preflight import preflight_cursor_hooks from apm_cli.integration.hook_file_routing import filter_hook_files_for_target from apm_cli.integration.hook_native_formats import ( _to_antigravity_hook_entries, _to_claude_hook_entries, _to_codex_hook_entries, + _to_cursor_hook_entries, _to_gemini_hook_entries, ) from apm_cli.integration.hook_ownership import ( @@ -126,17 +128,9 @@ class _MergeHookConfig: target_key: str # target name passed to _rewrite_hooks_data require_dir: bool # True = skip if target dir doesn't exist schema_strict: bool = True # Ownership always lives outside native files. - # Top-level JSON key the merged event map lives under. Defaults to - # "hooks" (Claude/Cursor/Codex/Gemini/Windsurf). Antigravity's native - # schema keys hooks by an arbitrary hook *name*, so APM reserves the - # single name "apm" as its container and leaves sibling user hook-names - # untouched. + # Antigravity reserves "apm" instead of "hooks"; sibling names belong to users. event_container_key: str = "hooks" - # Target-specific top-level keys to inject into the config file when - # absent. Used to emit required schema fields (e.g. "version": 1 for - # Cursor) that APM does not otherwise write. Existing keys are never - # overwritten -- the guard in _integrate_merged_hooks() preserves any - # value the user has set manually. + # Native defaults are injected only when absent, never over user values. top_level_defaults: dict[str, Any] = field(default_factory=dict) prompt_handler_types: tuple[str, ...] = () named_containers: bool = False @@ -169,6 +163,16 @@ class _MergeHookConfig: **dict.fromkeys(("SessionStart", "sessionStart"), "SessionStart"), **dict.fromkeys(("Stop", "AgentStop", "agentStop"), "Stop"), }, + "cursor": { # Values must stay within CURSOR_NATIVE_EVENTS. + "PreToolUse": "preToolUse", + "PostToolUse": "postToolUse", + "UserPromptSubmit": "beforeSubmitPrompt", + "Stop": "stop", + "SubagentStop": "subagentStop", + "SessionStart": "sessionStart", + "SessionEnd": "sessionEnd", + "PreCompact": "preCompact", + }, "gemini": { # Copilot / Claude -> Gemini "PreToolUse": "BeforeTool", @@ -211,7 +215,7 @@ class _MergeHookConfig: "copilot": "camelCase", "vscode": "PascalCase", "claude": "PascalCase", - "cursor": "PascalCase", + "cursor": "camelCase", "codex": "PascalCase", "gemini": "PascalCase", "antigravity": "PascalCase", @@ -319,6 +323,8 @@ def _validate_copilot_payload(payload: dict) -> list[str]: target_key="cursor", require_dir=True, top_level_defaults={"version": 1}, + prompt_handler_types=("prompt",), + nested_handlers=False, ), "codex": _MergeHookConfig( config_filename="hooks.json", @@ -1027,10 +1033,7 @@ def integrate_package_hooks( if not hook_files: return HookIntegrationResult( - files_integrated=0, - files_updated=0, - files_skipped=0, - target_paths=[], + files_integrated=0, files_updated=0, files_skipped=0, target_paths=[] ) root_dir = target.root_dir if target else ".github" @@ -1200,18 +1203,11 @@ def _integrate_merged_hooks( target=None, user_scope: bool = False, source_plan=None, + retiring_targets: frozenset[str] = frozenset(), ) -> HookIntegrationResult: - """Integrate hooks by merging into a target-specific JSON config. - - This is the shared implementation for Claude, Cursor, and Codex - targets that merge hook entries into a single JSON file (as - opposed to Copilot which uses individual JSON files). - """ + """Merge native hook entries into a shared target-specific JSON config.""" _empty = HookIntegrationResult( - files_integrated=0, - files_updated=0, - files_skipped=0, - target_paths=[], + files_integrated=0, files_updated=0, files_skipped=0, target_paths=[] ) root_dir = target.root_dir if target else f".{config.target_key}" @@ -1244,20 +1240,30 @@ def _integrate_merged_hooks( package_name, dependency_sources, ) + # Always re-check authorized Cursor/import sources at this write + # boundary, even if preflight_hooks_for_targets already ran for this + # plan. A plan object can be reused across projects or after a + # Claude import file is added mid-install; a cached "already + # preflighted" flag would silently bypass the check in exactly that + # case (#3129). Forward retiring_targets: this fallback also needs it. + if config.target_key in {"cursor", "claude"}: + preflight_cursor_hooks( + self, + package_info, + project_root, + hook_sources, + _HOOK_EVENT_MAP, + user_scope=user_scope, + retiring_targets=retiring_targets, + ) hooks_integrated = 0 scripts_copied = 0 scripts_adopted = 0 target_paths: list[Path] = [] display_payloads: list = [] - # Per-file display metadata is captured during the merge loop but - # the payloads are BUILT after the JSON config is finalized (Gemini - # transform applied, schema-strict _apm_source stripped) so that - # rendered_json reflects the actual on-disk/executed content. + # Finalize display JSON after native conversion and ownership extraction. pending_display: list = [] - # Events whose prior-owned entries have already been cleared on - # this install run. Packages can contribute to the same event - # from multiple hook files -- we must only strip once so earlier - # files' fresh entries aren't wiped by later iterations. + # Clear once so later files do not erase earlier contributions. cleared_events: set = set() # Read existing JSON config @@ -1293,18 +1299,23 @@ def _integrate_merged_hooks( if sidecar_data and container in json_config: _reinject_apm_source_from_sidecar(json_config[container], sidecar_data) - # Top-level container key for the merged event map. Most targets - # use "hooks"; Antigravity nests its events under the reserved - # hook-name "apm" so sibling user hook-names are preserved. Only - # the container key is created so non-"hooks" targets never gain a - # stray empty "hooks" object in their native file. + # Antigravity reserves "apm"; leave sibling user containers untouched. if container not in json_config: json_config[container] = {} _log.debug("Seeded hook container '%s' in %s", container, config.config_filename) - # Inject any target-specific top-level defaults (e.g. "version": 1 for - # Cursor) that are absent from the existing file. Existing values are - # never overwritten so a user-set "version" is preserved across reinstalls. + if config.target_key == "cursor": + for event in list(json_config[container]): + json_config[container][event] = [ + entry + for entry in json_config[container][event] + if not isinstance(entry, dict) + or entry.get("_apm_source") not in {source_marker, *legacy_source_markers} + ] + if not json_config[container][event]: + del json_config[container][event] + + # Defaults never overwrite existing user fields. injected_keys: list[str] = [] for key, value in config.top_level_defaults.items(): if key not in json_config: @@ -1354,6 +1365,10 @@ def _integrate_merged_hooks( legacy_content_keys: set[str] = set() if config.target_key == "claude": entries = _to_claude_hook_entries(entries) + elif config.target_key == "cursor": + entries = _to_cursor_hook_entries( + entries, event_name, foreign=raw_event_name in event_map + ) elif config.target_key == "codex": # Match owned flat entries from installs before Codex nesting. legacy_content_keys = { @@ -1459,12 +1474,7 @@ def _integrate_merged_hooks( deduped.append(entry) json_config[container][event_name] = deduped entries_appended_for_file = True - # Capture the actual entry objects this file contributed to - # the merged config. They are the same dict references that - # the schema-strict strip mutates in place below, so building - # the display payload from them after finalization yields - # rendered_json that matches the on-disk/executed content - # (Gemini-transformed, _apm_source stripped where required). + # Retain references so ownership stripping also cleans display JSON. file_event_entries.setdefault(event_name, []).extend( e for e in entries if isinstance(e, dict) ) @@ -1480,15 +1490,7 @@ def _integrate_merged_hooks( ) ) else: - # Diagnostic for the fail-closed silent-skip path introduced - # by the integrated-counter fix (microsoft/apm#1499): a hook - # file that parsed cleanly but contributed zero entries (all - # events empty / non-list) used to bump the counter and lie - # to the user. Now we skip it -- emit a user-visible warning - # (the original #1499 symptom was that authors saw nothing - # bad AND nothing good, so a structured-logger-only message - # would re-introduce the silent-failure UX) and a parallel - # _log.warning for operators consuming structured logs. + # Empty sources must not produce misleading success (#1499). rel_hook = hook_file.name _rich_warning( f"Hook file {rel_hook} contributed no entries to " @@ -1518,9 +1520,7 @@ def _integrate_merged_hooks( scripts_copied += copy_result.scripts_copied scripts_adopted += copy_result.files_adopted - # Write JSON config back - # Don't track the config file in target_paths -- it's a shared - # file cleaned via _apm_source markers, not file-level deletion + # Shared configs are cleaned by ownership, never file-level deletion. json_path.parent.mkdir(parents=True, exist_ok=True) if config.schema_strict: @@ -1536,10 +1536,7 @@ def _integrate_merged_hooks( elif sidecar_path.exists(): sidecar_path.unlink() - # Build display payloads from the finalized entry objects (post - # Gemini transform and post schema-strict _apm_source strip) so the - # CLI summary and rendered_json faithfully reflect what is written - # to disk and executed -- not the pre-transform per-file data. + # Display only finalized, schema-clean native entries. for _label, _path, _hook_file, _file_event_entries in pending_display: display_payloads.append( self._build_display_payload( @@ -1550,10 +1547,8 @@ def _integrate_merged_hooks( ) ) - # Write the (now schema-clean) config - with open(json_path, "w", encoding="utf-8") as f: - json.dump(json_config, f, indent=2) - f.write("\n") + # Cursor's strict config must not be observable half-written by its watcher. + atomic_write_text(json_path, json.dumps(json_config, indent=2) + "\n") return HookIntegrationResult( files_integrated=hooks_integrated, @@ -1637,6 +1632,28 @@ def integrate_package_hooks_codex( user_scope=user_scope, ) + def preflight_hooks_for_targets( + self, + package_info: Any, + project_root: Path, + source_plan: Any, + *, + user_scope: bool = False, + retiring_targets: frozenset[str] = frozenset(), + ) -> None: + """Check authorized Cursor/import sources before any primitive is written.""" + selection = source_plan.hook_source_selection + if selection is not None: + preflight_cursor_hooks( + self, + package_info, + project_root, + selection, + _HOOK_EVENT_MAP, + user_scope=user_scope, + retiring_targets=retiring_targets, + ) + def integrate_hooks_for_target( self, target, @@ -1651,6 +1668,7 @@ def integrate_hooks_for_target( dep_targets_active: bool = False, allowed_targets: set[str] | None = None, source_plan=None, + retiring_targets: frozenset[str] = frozenset(), ) -> "HookIntegrationResult": """Integrate hooks for a single *target*. @@ -1658,11 +1676,8 @@ def integrate_hooks_for_target( All other merge-based targets are dispatched via the ``_MERGE_HOOK_TARGETS`` registry. - ``user_scope`` controls whether merged-hook ``command`` paths are - rewritten to absolute paths (required when deploying to - ``~/.claude/settings.json`` -- see #1310 / #1354) or left - repo-relative so checked-in project-scope configs stay portable - across clones, contributors, and CI runners (#1394). + ``user_scope`` requires cwd-independent paths (#1310 / #1354); + project configs retain portable repo-relative paths (#1394). """ if dep_targets_active and (not allowed_targets or target.name not in allowed_targets): raise AssertionError(f"BUG: target {target.name} bypassed chokepoint filter") @@ -1705,6 +1720,7 @@ def integrate_hooks_for_target( target=target, user_scope=user_scope, source_plan=source_plan, + retiring_targets=retiring_targets, ) return HookIntegrationResult( diff --git a/src/apm_cli/integration/hook_native_formats.py b/src/apm_cli/integration/hook_native_formats.py index 994c12fa7..05e931628 100644 --- a/src/apm_cli/integration/hook_native_formats.py +++ b/src/apm_cli/integration/hook_native_formats.py @@ -2,6 +2,7 @@ from __future__ import annotations +import math from dataclasses import dataclass from typing import Any @@ -17,6 +18,195 @@ _ANTIGRAVITY_NESTED_EVENTS: frozenset[str] = frozenset({"PreToolUse", "PostToolUse"}) +# https://cursor.com/docs/hooks (native events, not case-derived aliases). +CURSOR_NATIVE_EVENTS = frozenset( + { + "sessionStart", + "sessionEnd", + "preToolUse", + "postToolUse", + "postToolUseFailure", + "subagentStart", + "subagentStop", + "beforeShellExecution", + "afterShellExecution", + "beforeMCPExecution", + "afterMCPExecution", + "beforeReadFile", + "afterFileEdit", + "beforeSubmitPrompt", + "preCompact", + "stop", + "afterAgentResponse", + "afterAgentThought", + "beforeTabFileRead", + "afterTabFileEdit", + "workspaceOpen", + } +) + +# https://cursor.com/docs/hooks - the only keys Cursor's loader recognizes. +CURSOR_CONFIG_TOP_LEVEL_KEYS = frozenset({"version", "hooks"}) + + +def _cursor_matcher(matcher: str | None, event: str, *, foreign: bool) -> str | None: + """Translate only explicitly representable Claude tool-name alternatives.""" + if matcher is not None and not isinstance(matcher, str): + raise HookContractError("Cursor matcher must be a string") + if not foreign or matcher in (None, "", "*", ".*"): + return matcher + if event not in {"preToolUse", "postToolUse"}: + raise HookContractError("Claude event matcher has no verified Cursor equivalent") + names = matcher.split("|") + mapping = { + "Bash": "Shell", + "Read": "Read", + "Edit": "Write", + "Write": "Write", + "Grep": "Grep", + "Task": "Task", + "WebFetch": "WebFetch", + "WebSearch": "WebSearch", + } + if any(name not in mapping for name in names): + if all(name.strip() in mapping for name in names): + raise HookContractError( + "Claude matcher alternatives contain whitespace; use literal alternatives " + "such as 'Bash|Read' instead of 'Bash | Read'" + ) + raise HookContractError( + "Cursor cannot preserve this Claude matcher; regex, Glob and server-qualified " + "MCP translations are not supported" + ) + # Cursor combines file creation and editing under Write. Mapping either + # Claude tool alone would broaden the original restriction. + if set(names) & {"Edit", "Write"} and not {"Edit", "Write"} <= set(names): + raise HookContractError("Cursor Write requires both Claude Edit and Write alternatives") + return "|".join(dict.fromkeys(mapping[name] for name in names)) + + +def _check_cursor_field_types(entry: dict[str, Any]) -> None: + """Keep source and rendered field predicates identical across IR conversion.""" + if "matcher" in entry and not isinstance(entry["matcher"], str): + raise HookContractError("Cursor matcher must be a string") + for key in ("timeout", "timeoutSec"): + if key in entry and ( + type(entry[key]) not in (int, float) or not math.isfinite(entry[key]) or entry[key] <= 0 + ): + raise HookContractError("Cursor timeout must be a finite positive number of seconds") + if "failClosed" in entry and type(entry["failClosed"]) is not bool: + raise HookContractError("Cursor failClosed must be a boolean") + + +def _validate_cursor_handler(entry: dict[str, Any], event: str) -> None: + """Validate native output; source-only checks live in _to_cursor_hook_entries.""" + allowed = { + "type", + "command", + "prompt", + "model", + "timeout", + "matcher", + "loop_limit", + "failClosed", + "_apm_source", + } + if entry.keys() - allowed: + raise HookContractError( + f"unsupported Cursor handler fields {sorted(entry.keys() - allowed)!r}; " + "no fields were discarded" + ) + _check_cursor_field_types(entry) + if "_apm_source" in entry and not isinstance(entry["_apm_source"], str): + raise HookContractError("invalid Cursor hook ownership metadata") + kind = entry.get("type", "command") + if not isinstance(kind, str) or kind not in {"command", "prompt"}: + raise HookContractError("Cursor supports only command and prompt handlers") + content_key = "prompt" if kind == "prompt" else "command" + incompatible = {"command"} if kind == "prompt" else {"prompt", "model"} + if incompatible & entry.keys() or not isinstance(entry.get(content_key), str): + raise HookContractError(f"Cursor {kind} handler requires a string {content_key}") + if "model" in entry and not isinstance(entry["model"], str): + raise HookContractError("Cursor prompt model must be a string") + if "loop_limit" in entry: + limit = entry["loop_limit"] + if event not in {"stop", "subagentStop"} or ( + limit is not None and (type(limit) is not int or limit < 0) + ): + raise HookContractError( + "Cursor loop_limit requires stop/subagentStop and integer or null" + ) + + +def _to_cursor_hook_entries( + entries: list, event_name: str, *, foreign: bool = False +) -> list[dict[str, Any]]: + """Render the neutral IR as strict native Cursor flat handlers. + + Claude import mappings are documented at + https://cursor.com/docs/reference/third-party-hooks. The bounded adapter + rejects mappings that lose restrictions instead of imitating lossy import. + Check source-only restrictions before IR normalization loses them, then + validate rendered output through _validate_cursor_handler. + """ + if event_name not in CURSOR_NATIVE_EVENTS: + raise HookContractError(f"unsupported Cursor event {event_name!r}") + for declaration in hook_handlers({"hooks": {event_name: entries}}): + raw = declaration.value + _check_cursor_field_types(raw) + if "timeout" in raw and "timeoutSec" in raw: + raise HookContractError("Cursor handler must declare only one timeout") + if ( + foreign + and "/hooks/" in declaration.json_pointer.removeprefix("/hooks/") + and "matcher" in raw + ): + raise HookContractError( + "Claude handler-level matcher has no verified Cursor equivalent" + ) + result: list[dict[str, Any]] = [] + for binding in _entries_to_ir(entries, event_name).bindings: + if binding.metadata: + raise HookContractError( + f"unsupported Cursor matcher-group fields {sorted(binding.metadata)!r}" + ) + matcher = _cursor_matcher(binding.matcher, event_name, foreign=foreign) + for handler in binding.handlers: + if handler.platform != "all": + raise HookContractError( + "Cursor cannot preserve platform-specific command restrictions" + ) + rendered = _handler_from_ir(handler, timeout_milliseconds=False) + if matcher is not None: + if "matcher" in rendered: + raise HookContractError("Cursor cannot combine nested and outer matchers") + rendered["matcher"] = matcher + if foreign and event_name in {"stop", "subagentStop"}: + rendered.setdefault("loop_limit", None) + _validate_cursor_handler(rendered, event_name) + result.append(rendered) + return result + + +def validate_cursor_config(document: object) -> None: + """Reject a native file that Cursor would not load, without repairing user data.""" + if not isinstance(document, dict) or not isinstance(document.get("hooks"), dict): + raise HookContractError("Cursor config requires a hooks object") + unknown_keys = document.keys() - CURSOR_CONFIG_TOP_LEVEL_KEYS + if unknown_keys: + raise HookContractError( + f"Cursor config has unsupported top-level keys {sorted(unknown_keys)!r}" + ) + if type(document.get("version")) is not int or document["version"] != 1: + raise HookContractError("Cursor config requires version 1") + for event, entries in document["hooks"].items(): + if event not in CURSOR_NATIVE_EVENTS or not isinstance(entries, list): + raise HookContractError(f"unsupported Cursor event {event!r} or non-array entries") + for entry in entries: + if not isinstance(entry, dict): + raise HookContractError("Cursor native handlers must be objects") + _validate_cursor_handler(entry, event) + @dataclass(frozen=True) class HookContentEntry: diff --git a/tests/integration/test_architecture_contract_guards.py b/tests/integration/test_architecture_contract_guards.py index 80f7a2c26..e57fc82a6 100644 --- a/tests/integration/test_architecture_contract_guards.py +++ b/tests/integration/test_architecture_contract_guards.py @@ -7,11 +7,49 @@ import pytest -from scripts.architecture_linter.runner import registered_rules +from scripts.architecture_linter.runner import registered_rules, run_selected_rules _RULES_BY_ID = {rule.id: rule for rule in registered_rules()} +@pytest.mark.parametrize( + ("path", "old", "new"), + [ + ( + "src/apm_cli/integration/hook_integrator.py", + "entries = _to_cursor_hook_entries(", + "entries = _local_cursor_renderer(", + ), + ( + "src/apm_cli/integration/hook_cursor_preflight.py", + "validate_cursor_config(candidate)", + "pass", + ), + ( + "src/apm_cli/install/services.py", + '"preflight_hooks_for_targets"', + '"unvalidated_hooks_for_targets"', + ), + ( + "src/apm_cli/integration/hook_integrator.py", + "_log = logging.getLogger(__name__)", + "_log = logging.getLogger(__name__)\nCURSOR_NATIVE_EVENTS = frozenset()", + ), + ], +) +def test_cursor_native_edge_boundary_rejects_bypass_and_second_owner( + path: str, old: str, new: str +) -> None: + root = Path(__file__).parents[2] + source = (root / path).read_text(encoding="utf-8") + assert old in source + rule = _RULES_BY_ID["mutation_writes.neutral_hook_contract"] + result = run_selected_rules( + root, (rule.id,), source_overrides={path: source.replace(old, new, 1)} + ) + assert any(finding.rule_id == rule.id for finding in result.violations) + + def _write_portable_hook_package(tmp_path: Path) -> object: """Create one package carrying the flat portable hook shape.""" from apm_cli.models.apm_package import APMPackage, PackageInfo diff --git a/tests/integration/test_architecture_owner_rule_mutations.py b/tests/integration/test_architecture_owner_rule_mutations.py index dc1c320ae..01007aa1f 100644 --- a/tests/integration/test_architecture_owner_rule_mutations.py +++ b/tests/integration/test_architecture_owner_rule_mutations.py @@ -1440,6 +1440,140 @@ def test_git_semver_guard_rejects_bypassing_selected_attempt_requested_url() -> ) +@pytest.mark.parametrize("argument", ["raw", "entry"]) +def test_neutral_hook_contract_guard_rejects_split_cursor_field_validation(argument: str) -> None: + """Both source and native-output validation must route through the same predicates.""" + path = "src/apm_cli/integration/hook_native_formats.py" + source = _source(path) + call = f"_check_cursor_field_types({argument})" + assert source.count(call) == 1 + mutated = source.replace(call, "pass", 1) + ast.parse(mutated, filename=path) + rule_id = "mutation_writes.neutral_hook_contract" + report = run_selected_rules(ROOT, (rule_id,), source_overrides={path: mutated}) + assert report.failures == () + assert any(violation.rule_id == rule_id for violation in report.violations) + + +def test_neutral_hook_contract_guard_rejects_cached_cursor_preflight_gate() -> None: + """A lexical "preflight call is present" check cannot catch a cache-gated + reintroduction of the #3129 reused-plan bypass: the literal + ``preflight_cursor_hooks(`` call stays present in the file whether or + not it is wrapped in an ``if not plan.cursor_preflight_done:`` guard. + This proves the dedicated token-forbidding sub-check (not just the + existing presence check) is the thing that actually catches that + specific regression shape. + """ + path = "src/apm_cli/integration/hook_integrator.py" + source = _source(path) + old = ( + ' if config.target_key in {"cursor", "claude"}:\n' + " preflight_cursor_hooks(\n" + ) + assert source.count(old) == 1 + mutated = source.replace( + old, + ( + ' if config.target_key in {"cursor", "claude"} and not getattr(\n' + ' source_plan, "cursor_preflight_done", False\n' + " ):\n" + " preflight_cursor_hooks(\n" + ), + 1, + ) + ast.parse(mutated, filename=path) + + rule_id = "mutation_writes.neutral_hook_contract" + report = run_selected_rules(ROOT, (rule_id,), source_overrides={path: mutated}) + + assert report.failures == () + assert any( + violation.rule_id == rule_id and "cursor_preflight_done" in violation.message + for violation in report.violations + ) + + +def test_neutral_hook_contract_guard_rejects_removed_preflight_call_site() -> None: + """A file-wide ``preflight_cursor_hooks(`` presence check cannot catch + the exact per-write call being deleted from + ``HookIntegrator._integrate_merged_hooks``: the separate, unrelated + upfront ``preflight_hooks_for_targets`` call site leaves that substring + present elsewhere in the file even after this one is removed. Only the + structural, call-site-specific sub-check catches this regression shape. + """ + path = "src/apm_cli/integration/hook_integrator.py" + source = _source(path) + old = ( + ' if config.target_key in {"cursor", "claude"}:\n' + " preflight_cursor_hooks(\n" + " self,\n" + " package_info,\n" + " project_root,\n" + " hook_sources,\n" + " _HOOK_EVENT_MAP,\n" + " user_scope=user_scope,\n" + " retiring_targets=retiring_targets,\n" + " )\n" + ) + assert source.count(old) == 1 + mutated = source.replace( + old, + ' if config.target_key in {"cursor", "claude"}:\n pass\n', + 1, + ) + assert "preflight_cursor_hooks(" in mutated, "unrelated call site must survive the mutation" + ast.parse(mutated, filename=path) + + rule_id = "mutation_writes.neutral_hook_contract" + report = run_selected_rules(ROOT, (rule_id,), source_overrides={path: mutated}) + + assert report.failures == () + assert any( + violation.rule_id == rule_id + and "direct, unconditional statement" in violation.message + and "preflight_cursor_hooks" in violation.message + for violation in report.violations + ) + + +def test_neutral_hook_contract_guard_rejects_renamed_cache_predicate_rewrap() -> None: + """The reused-plan bypass can be reintroduced under any predicate name, + not just ``cursor_preflight_done``. Nesting the per-write call in a + second conditional -- regardless of what the cache flag is called -- + must fail the structural call-site check even when the token-ban + sub-check has nothing to match. + """ + path = "src/apm_cli/integration/hook_integrator.py" + source = _source(path) + old = ( + ' if config.target_key in {"cursor", "claude"}:\n' + " preflight_cursor_hooks(\n" + ) + assert source.count(old) == 1 + mutated = source.replace( + old, + ( + ' if config.target_key in {"cursor", "claude"}:\n' + ' if not getattr(source_plan, "_already_rendered_once", False):\n' + " preflight_cursor_hooks(\n" + ), + 1, + ) + assert "cursor_preflight_done" not in mutated + ast.parse(mutated, filename=path) + + rule_id = "mutation_writes.neutral_hook_contract" + report = run_selected_rules(ROOT, (rule_id,), source_overrides={path: mutated}) + + assert report.failures == () + assert any( + violation.rule_id == rule_id + and "direct, unconditional statement" in violation.message + and "preflight_cursor_hooks" in violation.message + for violation in report.violations + ) + + @pytest.mark.parametrize( ("old", "new"), [ diff --git a/tests/integration/test_cursor_hook_lifecycle.py b/tests/integration/test_cursor_hook_lifecycle.py new file mode 100644 index 000000000..38a347048 --- /dev/null +++ b/tests/integration/test_cursor_hook_lifecycle.py @@ -0,0 +1,110 @@ +"""Installed CLI proof of Cursor native hooks and safe import coexistence.""" + +from __future__ import annotations + +import json +import os +import shlex +import subprocess +import sys +from pathlib import Path + +import pytest + +from tests.utils.apm_lifecycle_runner import ApmLifecycleRunner +from tests.utils.isolated_apm_environment import IsolatedApmEnvironment +from tests.utils.local_package import LocalPackageFactory + +pytestmark = [pytest.mark.e2e, pytest.mark.lifecycle_smoke, pytest.mark.requires_apm_binary] + + +@pytest.mark.parametrize("shared", [False, True], ids=["native-lifecycle", "import-conflict"]) +def test_cursor_installed_cli_contract(tmp_path: Path, apm_binary_path: Path, shared: bool) -> None: + isolated = IsolatedApmEnvironment.create(tmp_path / "cursor", base_env=dict(os.environ)) + environment = isolated.subprocess_env() + sources = LocalPackageFactory(isolated.package_root) + consumers = LocalPackageFactory(isolated.work_root) + targets = ("claude", "cursor") if shared else ("cursor",) + package = sources.create("cursor-native", targets=targets) + source = package.root / ".apm/hooks/probe.py" + source.parent.mkdir(parents=True) + source.write_text('print(\'{"permission":"allow"}\')\n', encoding="utf-8") + sources.add_hook( + package, + "gate", + { + "hooks": { + "PreToolUse": [ + { + "matcher": "Bash", + "hooks": [ + { + "type": "command", + "command": f'"{sys.executable}" "${{PLUGIN_ROOT}}/.apm/hooks/probe.py"', + "timeout": 10, + } + ], + } + ] + }, + }, + ) + consumer = consumers.create( + "consumer", + dependencies=({"path": str(package.root)},), + targets=targets, + ) + config = consumer.root / ".cursor/hooks.json" + config.parent.mkdir() + user = {"version": 1, "hooks": {"afterFileEdit": [{"command": "echo user"}]}} + config.write_text(json.dumps(user) + "\n", encoding="utf-8") + before = config.read_bytes() + runner = ApmLifecycleRunner((str(apm_binary_path),)) + args = ("install", "--no-policy", "--parallel-downloads", "0") + first = runner.run(args, scenario_id="cursor-install", cwd=consumer.root, env=environment) + if shared: + assert first.returncode != 0, first.stdout + first.stderr + # Rich wraps CLI error output to the CI runner's (often narrower, + # non-TTY) detected console width, which can split the literal + # "Claude import" substring across a line break. Collapse all + # whitespace runs (including embedded wrap newlines) before the + # substring check so the assertion verifies the full semantic + # phrase regardless of wrap point, instead of depending on a + # specific terminal width. + normalized_output = " ".join((first.stdout + first.stderr).split()) + assert "Claude import" in normalized_output + assert config.read_bytes() == before + assert not (consumer.root / ".claude/settings.json").exists() + assert not config.with_name("apm-hooks.json").exists() + return + assert first.returncode == 0, first.stdout + first.stderr + native = json.loads(config.read_text()) + assert set(native["hooks"]) == {"preToolUse", "afterFileEdit"} + entry = native["hooks"]["preToolUse"][0] + assert entry["matcher"] == "Shell" + assert "hooks" not in entry + executed = subprocess.run( + shlex.split(entry["command"]), + cwd=consumer.root, + env=environment, + capture_output=True, + text=True, + check=False, + timeout=10, + ) + assert executed.returncode == 0, executed.stderr + assert json.loads(executed.stdout) == {"permission": "allow"} + first_bytes = config.read_bytes() + second = runner.run(args, scenario_id="cursor-reinstall", cwd=consumer.root, env=environment) + assert second.returncode == 0, second.stdout + second.stderr + assert config.read_bytes() == first_bytes + + removed = runner.run( + ("uninstall", str(package.root)), + scenario_id="cursor-uninstall", + cwd=consumer.root, + env=environment, + ) + assert removed.returncode == 0, removed.stdout + removed.stderr + assert json.loads(config.read_text()) == user + assert not config.with_name("apm-hooks.json").exists() diff --git a/tests/integration/test_hook_target_contraction_reconciliation.py b/tests/integration/test_hook_target_contraction_reconciliation.py index 42563b59c..581bcfc5c 100644 --- a/tests/integration/test_hook_target_contraction_reconciliation.py +++ b/tests/integration/test_hook_target_contraction_reconciliation.py @@ -215,10 +215,11 @@ def _pre_tool_use_commands(config_path: Path) -> list[str]: if not config_path.exists(): return [] data = _read_json(config_path) - entries = data.get("hooks", {}).get("PreToolUse", []) + hooks = data.get("hooks", {}) + entries = hooks.get("preToolUse" if config_path.parent.name == ".cursor" else "PreToolUse", []) commands = [] for entry in entries: - for handler in entry.get("hooks", []): + for handler in entry.get("hooks", [entry]): if isinstance(handler, dict) and "command" in handler: commands.append(handler["command"]) return commands @@ -404,7 +405,7 @@ def test_widen_then_narrow_removes_dropped_cursor_hook_state( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: """Generic-owner proof, Cursor variant (required-gate node-2 evidence): - Claude -> Claude+Cursor -> Claude (widen then narrow back, not just a + Codex -> Codex+Cursor -> Codex (widen then narrow back, not just a single narrow from an initial multi-target install) previously left `.cursor/hooks.json` and its `apm-hooks.json` ownership sidecar fully intact with the dropped target's marker, even when Cursor's own @@ -414,17 +415,17 @@ def test_widen_then_narrow_removes_dropped_cursor_hook_state( GREEN (post-fix): `HookIntegrator.reconcile_dropped_targets` is generic over `_MERGE_HOOK_TARGETS` (Claude, Cursor, Codex, Gemini, Windsurf, - Antigravity) -- narrowing back to Claude-only reconciles Cursor's + Antigravity) -- narrowing back to Codex-only reconciles Cursor's merge-hook state exactly like Codex's, asserted directly against the native merged config and the sidecar here rather than relying on any audit/orphan-detection surface (which only reports the separate, out-of-scope `.agents/skills` target-row defect).""" project = tmp_path / "proj-cursor-widen-narrow" - _write_project(project, ["acme/pkg-a"], ["claude"]) + _write_project(project, ["acme/pkg-a"], ["codex"]) result = _run_install(project, monkeypatch, {"acme/pkg-a": "./scripts/pkg-a-hook.sh"}) assert result.exit_code == 0, result.output - _narrow_targets(project, ["claude", "cursor"]) # widen + _narrow_targets(project, ["codex", "cursor"]) # widen without a Claude-import route widen_result = _run_install(project, monkeypatch, {"acme/pkg-a": "./scripts/pkg-a-hook.sh"}) assert widen_result.exit_code == 0, widen_result.output @@ -437,7 +438,8 @@ def test_widen_then_narrow_removes_dropped_cursor_hook_state( "precondition: cursor native config carries the widened install's hook command" ) - _narrow_targets(project, ["claude"]) # narrow back + codex_before = (project / ".codex" / "hooks.json").read_bytes() + _narrow_targets(project, ["codex"]) # drop Cursor, retain Codex narrow_result = _run_install(project, monkeypatch, {"acme/pkg-a": "./scripts/pkg-a-hook.sh"}) assert narrow_result.exit_code == 0, narrow_result.output @@ -455,6 +457,7 @@ def test_widen_then_narrow_removes_dropped_cursor_hook_state( "dropped cursor target's dead hook command must not survive in the native " "merged config, asserted directly, not via audit" ) + assert (project / ".codex" / "hooks.json").read_bytes() == codex_before def test_widen_then_narrow_preserves_user_owned_cursor_entries( @@ -465,29 +468,27 @@ def test_widen_then_narrow_preserves_user_owned_cursor_entries( proving the generic owner's marker-based ownership check -- not a per-target special case -- gates the deletion.""" project = tmp_path / "proj-cursor-user-owned" - _write_project(project, ["acme/pkg-a"], ["claude"]) + _write_project(project, ["acme/pkg-a"], ["codex"]) result = _run_install(project, monkeypatch, {"acme/pkg-a": "./scripts/pkg-a-hook.sh"}) assert result.exit_code == 0, result.output - _narrow_targets(project, ["claude", "cursor"]) # widen + _narrow_targets(project, ["codex", "cursor"]) # widen without a Claude-import route widen_result = _run_install(project, monkeypatch, {"acme/pkg-a": "./scripts/pkg-a-hook.sh"}) assert widen_result.exit_code == 0, widen_result.output cursor_settings = project / ".cursor" / "hooks.json" data = _read_json(cursor_settings) - data.setdefault("hooks", {}).setdefault("PreToolUse", []).append( - {"matcher": "Bash", "hooks": [{"type": "command", "command": "echo manual-cursor-hook"}]} - ) + manual = {"matcher": "Shell", "type": "command", "command": "echo manual-cursor-hook"} + data["hooks"]["preToolUse"].append(manual) cursor_settings.write_text(json.dumps(data), encoding="utf-8") - _narrow_targets(project, ["claude"]) # narrow back + _narrow_targets(project, ["codex"]) # drop Cursor, retain Codex narrow_result = _run_install(project, monkeypatch, {"acme/pkg-a": "./scripts/pkg-a-hook.sh"}) assert narrow_result.exit_code == 0, narrow_result.output - assert "echo manual-cursor-hook" in _pre_tool_use_commands(cursor_settings), ( - "user-owned cursor entry must survive dropped-target reconciliation" - ) + assert _read_json(cursor_settings) == {"version": 1, "hooks": {"preToolUse": [manual]}} assert "acme/pkg-a" not in _sidecar_sources(project / ".cursor" / "apm-hooks.json") + assert not (project / ".cursor" / "apm-hooks.json").exists() def test_dry_run_install_does_not_reconcile_dropped_target_state( diff --git a/tests/integration/test_hook_wipe_target_scope_e2e.py b/tests/integration/test_hook_wipe_target_scope_e2e.py index 267fdbe3b..34ea571c5 100644 --- a/tests/integration/test_hook_wipe_target_scope_e2e.py +++ b/tests/integration/test_hook_wipe_target_scope_e2e.py @@ -10,9 +10,9 @@ ``resolve_targets(explicit_target=apm_package.canonical_targets or None)`` -- the project's *currently declared* ``targets:`` list. -If a project's ``targets:`` list is narrowed (e.g. ``[claude, cursor]`` -> +If a project's ``targets:`` list is narrowed (e.g. ``[claude, codex]`` -> ``[claude]``) while a dependency stays installed and its old harness's -merged-hook file (``.cursor/hooks.json``) still exists on disk, the next +merged-hook file (``.codex/hooks.json``) still exists on disk, the next ``apm uninstall`` or ``apm prune`` run wiped that harness's entire ``_apm_source``-tagged entry set and deleted its ``apm-hooks.json`` sidecar -- including entries for packages that are still declared and @@ -33,7 +33,7 @@ the defective primitive (``uninstall``, ``prune``) crossed with the two merge-hook schemas already established as semantically distinct in ``test_prune_hook_reconciliation_e2e.py`` (Claude's schema-strict inline -sidecar-only ownership vs Cursor's ``require_dir`` layout) -- a third +sidecar-only ownership vs Codex's ``require_dir`` layout) -- a third caller or harness would not exercise new code, only repeat assertions. """ @@ -65,7 +65,7 @@ _TARGET_LAYOUT = { "claude": (".claude/settings.json", ".claude/apm-hooks.json"), - "cursor": (".cursor/hooks.json", ".cursor/apm-hooks.json"), + "codex": (".codex/hooks.json", ".codex/apm-hooks.json"), } # The two callers that route through HookIntegrator.sync_integration()'s @@ -241,8 +241,8 @@ def _pre_tool_use_commands(config_path: Path) -> list[str]: def _install_two_packages_two_targets(project: Path, monkeypatch: pytest.MonkeyPatch) -> None: - """Install pkg-a + pkg-b under [claude, cursor], both with merged hooks.""" - _write_project(project, ["acme/pkg-a", "acme/pkg-b"], ["claude", "cursor"]) + """Install pkg-a + pkg-b under [claude, codex], both with merged hooks.""" + _write_project(project, ["acme/pkg-a", "acme/pkg-b"], ["claude", "codex"]) result = _run_install( project, monkeypatch, @@ -263,38 +263,38 @@ def test_narrowing_targets_preserves_sibling_hooks_in_dropped_target( ) -> None: """RED (pre-fix): narrowing targets: then removing an unrelated package wiped EVERY package's entries (including still-installed pkg-a's) from - the dropped harness (cursor), and deleted its ownership sidecar. + the dropped harness (codex), and deleted its ownership sidecar. GREEN (post-fix): the still-declared, still-installed sibling's hooks and manual/unowned entries in the now-undeclared harness survive, because the wipe is scoped to the same resolved target set the - rebuild uses (the currently-declared [claude] only) -- cursor is + rebuild uses (the currently-declared [claude] only) -- codex is simply never touched by this operation. """ project = tmp_path / f"proj-{caller}" _install_two_packages_two_targets(project, monkeypatch) - cursor_settings = project / ".cursor" / "hooks.json" - cursor_sidecar = project / ".cursor" / "apm-hooks.json" + codex_settings = project / ".codex" / "hooks.json" + codex_sidecar = project / ".codex" / "apm-hooks.json" claude_settings = project / ".claude" / "settings.json" claude_sidecar = project / ".claude" / "apm-hooks.json" - assert {"acme/pkg-a", "acme/pkg-b"} <= _sidecar_sources(cursor_sidecar), ( - "precondition: both packages merged into cursor" + assert {"acme/pkg-a", "acme/pkg-b"} <= _sidecar_sources(codex_sidecar), ( + "precondition: both packages merged into codex" ) assert {"acme/pkg-a", "acme/pkg-b"} <= _sidecar_sources(claude_sidecar), ( "precondition: both packages merged into claude" ) - # Inject a manual, user-owned entry directly into cursor's native + # Inject a manual, user-owned entry directly into codex's native # file -- never in the sidecar, so it carries no _apm_source marker. # It must survive regardless of what happens to package-owned entries. - cursor_data = _read_json(cursor_settings) - cursor_data.setdefault("hooks", {}).setdefault("PreToolUse", []).append( - {"matcher": "Bash", "hooks": [{"type": "command", "command": "echo manual-cursor-hook"}]} + codex_data = _read_json(codex_settings) + codex_data.setdefault("hooks", {}).setdefault("PreToolUse", []).append( + {"matcher": "Bash", "hooks": [{"type": "command", "command": "echo manual-codex-hook"}]} ) - cursor_settings.write_text(json.dumps(cursor_data), encoding="utf-8") + codex_settings.write_text(json.dumps(codex_data), encoding="utf-8") - # Narrow targets: [claude, cursor] -> [claude]. Both deps stay installed. + # Narrow targets: [claude, codex] -> [claude]. Both deps stay installed. _narrow_targets(project, ["claude"]) result = _remove_package(caller, project, monkeypatch, "acme/pkg-b") @@ -305,16 +305,16 @@ def test_narrowing_targets_preserves_sibling_hooks_in_dropped_target( "sibling package declared under the narrowed targets: must survive" ) - # The now-undeclared harness (cursor) must be untouched entirely: its + # The now-undeclared harness (codex) must be untouched entirely: its # config file, sidecar, package-owned entries (both packages), and the # manual entry must all still be present exactly as before. - assert cursor_settings.exists(), "cursor config must not be deleted by a narrowed-away wipe" - assert cursor_sidecar.exists(), "cursor's ownership sidecar must survive" - assert {"acme/pkg-a", "acme/pkg-b"} <= _sidecar_sources(cursor_sidecar), ( - "neither package's cursor hook entry may be lost when cursor is out of scope" + assert codex_settings.exists(), "codex config must not be deleted by a narrowed-away wipe" + assert codex_sidecar.exists(), "codex's ownership sidecar must survive" + assert {"acme/pkg-a", "acme/pkg-b"} <= _sidecar_sources(codex_sidecar), ( + "neither package's codex hook entry may be lost when codex is out of scope" ) - assert "echo manual-cursor-hook" in _pre_tool_use_commands(cursor_settings), ( - "manual user-owned cursor hook must remain untouched" + assert "echo manual-codex-hook" in _pre_tool_use_commands(codex_settings), ( + "manual user-owned codex hook must remain untouched" ) # The currently-declared harness (claude) IS in scope: pkg-b's entry @@ -333,7 +333,7 @@ def test_negative_twin_without_narrowing_still_cleans_removed_target( ) -> None: """Negative twin: without narrowing, both harnesses stay in scope. - Removing pkg-b while targets: still declares [claude, cursor] must + Removing pkg-b while targets: still declares [claude, codex] must still reconcile pkg-b's entries out of BOTH harnesses (not just the declared subset) -- this proves the fix does not regress the normal, still-in-scope wipe/rebuild case into a silent no-op. @@ -341,13 +341,13 @@ def test_negative_twin_without_narrowing_still_cleans_removed_target( project = tmp_path / f"proj-negative-{caller}" _install_two_packages_two_targets(project, monkeypatch) - cursor_sidecar = project / ".cursor" / "apm-hooks.json" + codex_sidecar = project / ".codex" / "apm-hooks.json" claude_sidecar = project / ".claude" / "apm-hooks.json" result = _remove_package(caller, project, monkeypatch, "acme/pkg-b") assert result.exit_code == 0, result.output - for sidecar, label in ((cursor_sidecar, "cursor"), (claude_sidecar, "claude")): + for sidecar, label in ((codex_sidecar, "codex"), (claude_sidecar, "claude")): sources = _sidecar_sources(sidecar) assert "acme/pkg-b" not in sources, ( f"pkg-b's {label} entry must be reconciled when in scope" @@ -367,20 +367,20 @@ def test_target_scope_reconciliation_is_idempotent( first = _remove_package(caller, project, monkeypatch, "acme/pkg-b") assert first.exit_code == 0, first.output - cursor_sidecar = project / ".cursor" / "apm-hooks.json" + codex_sidecar = project / ".codex" / "apm-hooks.json" claude_settings = project / ".claude" / "settings.json" claude_sidecar = project / ".claude" / "apm-hooks.json" state_after_first = ( _read_json(claude_settings) if claude_settings.exists() else None, - cursor_sidecar.exists(), - _sidecar_sources(cursor_sidecar), + codex_sidecar.exists(), + _sidecar_sources(codex_sidecar), _sidecar_sources(claude_sidecar), ) # Second run: uninstall re-invoking the same package is a no-op/error # for `uninstall` (package already gone from apm.yml); prune with # nothing orphaned is a no-op. Both must succeed without further - # mutating cursor or claude state. + # mutating codex or claude state. if caller == "uninstall": second = _run_cli(project, monkeypatch, ["prune"]) else: @@ -389,8 +389,8 @@ def test_target_scope_reconciliation_is_idempotent( state_after_second = ( _read_json(claude_settings) if claude_settings.exists() else None, - cursor_sidecar.exists(), - _sidecar_sources(cursor_sidecar), + codex_sidecar.exists(), + _sidecar_sources(codex_sidecar), _sidecar_sources(claude_sidecar), ) assert state_after_second == state_after_first, "second run must be a no-op" @@ -410,11 +410,11 @@ def test_zero_write_abort_before_destructive_wipe_on_prune( claude_settings = project / ".claude" / "settings.json" claude_sidecar = project / ".claude" / "apm-hooks.json" - cursor_settings = project / ".cursor" / "hooks.json" - cursor_sidecar = project / ".cursor" / "apm-hooks.json" + codex_settings = project / ".codex" / "hooks.json" + codex_sidecar = project / ".codex" / "apm-hooks.json" before = { p: _read_json(p) if p.exists() else None - for p in (claude_settings, claude_sidecar, cursor_settings, cursor_sidecar) + for p in (claude_settings, claude_sidecar, codex_settings, codex_sidecar) } _remove_dependency_from_manifest(project, "acme/pkg-b") @@ -430,7 +430,7 @@ def test_zero_write_abort_before_destructive_wipe_on_prune( ) after = { p: _read_json(p) if p.exists() else None - for p in (claude_settings, claude_sidecar, cursor_settings, cursor_sidecar) + for p in (claude_settings, claude_sidecar, codex_settings, codex_sidecar) } assert after == before, ( "a failed target resolution must abort before any wipe -- no merged-hook " @@ -458,8 +458,8 @@ def test_scope_symmetry_is_load_bearing_mutation_break( _install_two_packages_two_targets(project, monkeypatch) _narrow_targets(project, ["claude"]) - cursor_sidecar = project / ".cursor" / "apm-hooks.json" - assert {"acme/pkg-a", "acme/pkg-b"} <= _sidecar_sources(cursor_sidecar) + codex_sidecar = project / ".codex" / "apm-hooks.json" + assert {"acme/pkg-a", "acme/pkg-b"} <= _sidecar_sources(codex_sidecar) original_sync_integration = HookIntegrator.sync_integration @@ -475,9 +475,9 @@ def _unscoped_sync_integration(self, apm_package, project_root, managed_files=No result = _remove_package("prune", project, monkeypatch, "acme/pkg-b") assert result.exit_code == 0, result.output - assert "acme/pkg-a" not in _sidecar_sources(cursor_sidecar), ( + assert "acme/pkg-a" not in _sidecar_sources(codex_sidecar), ( "with the targets= scope neutered (simulating the pre-#2250 unscoped " - "wipe), pkg-a's still-installed cursor hook entry must be gone -- " + "wipe), pkg-a's still-installed codex hook entry must be gone -- " "proving the real scoped call is what actually preserves it in " "test_narrowing_targets_preserves_sibling_hooks_in_dropped_target" ) diff --git a/tests/integration/test_install_cli_cursor_claude_import_recheck_e2e.py b/tests/integration/test_install_cli_cursor_claude_import_recheck_e2e.py new file mode 100644 index 000000000..bf0e70458 --- /dev/null +++ b/tests/integration/test_install_cli_cursor_claude_import_recheck_e2e.py @@ -0,0 +1,307 @@ +"""CLI/InstallCommand-tier coverage for the write-boundary re-check (#3129). + +``HookIntegrator._integrate_merged_hooks`` unconditionally re-checks +authorized Cursor/Claude-import sources at the per-target write boundary, +even though ``preflight_hooks_for_targets`` already ran up front for the +same plan -- specifically to catch a Claude import that appears between +those two steps. ``tests/unit/integration/test_cursor_hook_native_contract.py`` +already proves this holds at the ``HookIntegrator`` tier directly and at the +``services.integrate_package_primitives`` orchestration tier. + +Neither of those goes through the real ``apm install`` CLI command, which +has its own argument parsing, error rendering, and exit-code mapping on top +of the orchestration call. This module closes that last tier: it drives the +REAL ``apm install`` Click command end-to-end (``CliRunner``, no internals +called directly), stubbing only the network download seam +(``GitHubPackageDownloader.download_package``) -- matching the hermetic +pattern already used by ``tests/integration/test_hook_wipe_target_scope_e2e.py`` +and ``tests/integration/test_prune_hook_reconciliation_e2e.py``. +""" + +from __future__ import annotations + +import contextlib +import json +from datetime import datetime +from pathlib import Path +from unittest.mock import patch + +import pytest +import yaml +from click.testing import CliRunner + +from apm_cli.cli import cli +from apm_cli.deps.github_downloader import GitHubPackageDownloader +from apm_cli.integration import hook_integrator +from apm_cli.integration.hook_integrator import HookIntegrator +from apm_cli.models.apm_package import ( + APMPackage, + GitReferenceType, + PackageInfo, + ResolvedReference, + clear_apm_yml_cache, +) +from apm_cli.models.dependency.reference import DependencyReference + +pytestmark = [pytest.mark.integration] + +_PATCH_UPDATES = "apm_cli.commands._helpers.check_for_updates" +_DEP_REPO_URL = "acme/cursor-gate" + + +def _stub_download_package(_self, repo_ref, install_path, *_args, **_kwargs): + """Materialize a hermetic local package with one Cursor-compatible hook.""" + dep_ref = ( + repo_ref + if isinstance(repo_ref, DependencyReference) + else DependencyReference.parse(str(repo_ref)) + ) + install_path = Path(install_path) + install_path.mkdir(parents=True, exist_ok=True) + pkg_name = dep_ref.repo_url.rsplit("/", maxsplit=1)[-1] + (install_path / "apm.yml").write_text( + yaml.safe_dump( + {"name": pkg_name, "version": "1.0.0", "description": "Hermetic CLI-tier fixture"}, + sort_keys=False, + ), + encoding="utf-8", + ) + hooks_dir = install_path / ".apm" / "hooks" + hooks_dir.mkdir(parents=True, exist_ok=True) + (hooks_dir / "gate.json").write_text( + json.dumps( + { + "hooks": { + "PreToolUse": [ + { + "matcher": "Bash", + "hooks": [{"type": "command", "command": "echo shared"}], + } + ] + } + } + ), + encoding="utf-8", + ) + package = APMPackage.from_apm_yml(install_path / "apm.yml") + return PackageInfo( + package=package, + install_path=install_path, + installed_at=datetime.now().isoformat(), + dependency_ref=dep_ref, + resolved_reference=ResolvedReference( + original_ref="main", + ref_type=GitReferenceType.BRANCH, + resolved_commit=None, + ref_name="main", + ), + ) + + +def _write_project(project: Path) -> None: + project.mkdir(parents=True, exist_ok=True) + (project / "apm.yml").write_text( + yaml.safe_dump( + { + "name": "cli-tier-consumer", + "version": "1.0.0", + "targets": ["cursor"], + "dependencies": {"apm": [_DEP_REPO_URL]}, + }, + sort_keys=False, + ), + encoding="utf-8", + ) + clear_apm_yml_cache() + + +def _run_install(project: Path, monkeypatch: pytest.MonkeyPatch): + monkeypatch.chdir(project) + with ( + patch(_PATCH_UPDATES, return_value=None), + patch.object( + GitHubPackageDownloader, + "download_package", + autospec=True, + side_effect=_stub_download_package, + ), + ): + return CliRunner().invoke(cli, ["install", "--no-policy"], catch_exceptions=False) + + +def _run_install_with_late_claude_import( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + label: str, + *, + disable_per_write_recheck: bool = False, +): + """Run one real ``apm install`` CLI invocation, injecting a conflicting + Claude import between the CLI's real up-front preflight and its real + per-target write (both inside that single real command). + + When ``disable_per_write_recheck`` is set, the per-write re-check inside + ``HookIntegrator._integrate_merged_hooks`` is disabled for the duration + of that one real method call ONLY -- the up-front + ``preflight_hooks_for_targets`` call (which happens earlier, including + its own internal real ``preflight_cursor_hooks`` call) always runs for + real and is never touched by this scoping. This mirrors the + service-tier mutation proof in + ``tests/unit/integration/test_cursor_hook_native_contract.py`` at the + CLI tier, so the same "which call enforces the guarantee" question is + answered through the real command, not just the orchestration call. + """ + project = tmp_path / label / "project" + _write_project(project) + + real_preflight = HookIntegrator.preflight_hooks_for_targets + claude_path = project / ".claude/settings.json" + injected_claude_bytes = ( + json.dumps( + {"hooks": {"PreToolUse": [{"hooks": [{"type": "command", "command": "echo shared"}]}]}} + ) + + "\n" + ).encode("utf-8") + + def _preflight_then_inject_claude_import(self, *args, **kwargs): + real_preflight(self, *args, **kwargs) + claude_path.parent.mkdir(parents=True, exist_ok=True) + claude_path.write_bytes(injected_claude_bytes) + + real_integrate_merged = HookIntegrator._integrate_merged_hooks + + def _integrate_merged_with_recheck_disabled(self, *args, **kwargs): + original = hook_integrator.preflight_cursor_hooks + hook_integrator.preflight_cursor_hooks = lambda *a, **k: None + try: + return real_integrate_merged(self, *args, **kwargs) + finally: + hook_integrator.preflight_cursor_hooks = original + + with contextlib.ExitStack() as stack: + stack.enter_context( + patch.object( + HookIntegrator, "preflight_hooks_for_targets", _preflight_then_inject_claude_import + ) + ) + if disable_per_write_recheck: + stack.enter_context( + patch.object( + HookIntegrator, + "_integrate_merged_hooks", + _integrate_merged_with_recheck_disabled, + ) + ) + result = _run_install(project, monkeypatch) + + return result, project, claude_path, injected_claude_bytes + + +def _assert_cli_rejected_the_late_import( + result, project: Path, claude_path: Path, injected_bytes: bytes +) -> None: + """The shared regression assertion, reused unmodified against a real run + and a scoped-mutant run so the mutant proof exercises the exact same + check the baseline must satisfy. + + Rich wraps CLI error output to the CI runner's (often narrower, + non-TTY) detected console width, which can split the literal "Claude + import" substring across a line break. Collapse all whitespace runs + (including embedded wrap newlines) before the substring check, matching + this PR's existing narrow-console normalization, so the assertion + verifies the full semantic phrase regardless of wrap point. + """ + normalized_output = " ".join(result.output.split()) + assert result.exit_code != 0, ( + "expected a non-zero exit for a Claude import appearing mid-install; " + f"got 0 (missing refusal). output={normalized_output!r}" + ) + assert "Claude import" in normalized_output, normalized_output + assert not (project / ".cursor/hooks.json").exists(), ( + "native write must not occur when the rejection fires" + ) + assert claude_path.read_bytes() == injected_bytes + + +def test_cli_install_rejects_claude_import_appearing_mid_run( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A Claude import appearing between the CLI's real up-front preflight + and its real per-target write must be refused by the real command, with + the native target untouched and the injected import's bytes unchanged. + """ + result, project, claude_path, injected_bytes = _run_install_with_late_claude_import( + tmp_path, monkeypatch, "baseline" + ) + _assert_cli_rejected_the_late_import(result, project, claude_path, injected_bytes) + + +def test_cli_install_writer_only_disable_loses_the_rejection_then_restores( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Mutation control at the CLI tier: disabling ONLY the per-write + re-check inside ``_integrate_merged_hooks`` (not the up-front preflight, + which keeps running for real) must make the SAME regression assertion + above FAIL through the real ``apm install`` command, for the expected + missing-refusal reason -- then restoring the real guard must make a + fresh real run pass again. + + LOAD-BEARING: this is the test that proves, through the real ``apm + install`` CLI command specifically, that the CLI-tier refusal is + causally tied to the real per-write guard call rather than to an + incidental side effect of the fixture -- using the same + baseline/mutant/restored assertion pattern as the service-tier and + native-contract mutation tests in this suite. Other tests in the suite + (static architecture guards, service-tier mutation tests) also reject + deletion of that call; this test's distinct value is proving the + causal link survives at the CLI entry point, not that it is the + suite's sole detector of the deletion. + """ + # Baseline sanity check first, with every real guard intact. + result, project, claude_path, injected_bytes = _run_install_with_late_claude_import( + tmp_path, monkeypatch, "before" + ) + _assert_cli_rejected_the_late_import(result, project, claude_path, injected_bytes) + + # Scoped mutant: only the per-write recheck is disabled, for the + # duration of that one real method call. The real CLI's up-front + # preflight still runs for real. + mutant_result, mutant_project, mutant_claude_path, mutant_injected_bytes = ( + _run_install_with_late_claude_import( + tmp_path, monkeypatch, "mutant", disable_per_write_recheck=True + ) + ) + with pytest.raises(AssertionError, match="missing refusal"): + _assert_cli_rejected_the_late_import( + mutant_result, mutant_project, mutant_claude_path, mutant_injected_bytes + ) + # Confirm the assertion failed for the expected reason (the real CLI + # command exited successfully and wrote the native target), not some + # unrelated break. + assert mutant_result.exit_code == 0, mutant_result.output + assert (mutant_project / ".cursor/hooks.json").exists() + + # Restored: the real guard is back in place for the next real install. + restored_result, restored_project, restored_claude_path, restored_injected_bytes = ( + _run_install_with_late_claude_import(tmp_path, monkeypatch, "restored") + ) + _assert_cli_rejected_the_late_import( + restored_result, restored_project, restored_claude_path, restored_injected_bytes + ) + + +def test_cli_install_without_a_late_import_succeeds( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Nonconflicting control: the same real CLI install, with no import + injected, must still succeed and write the native Cursor target -- this + rules out the refusal above being a false positive baked into the + fixture rather than a real reaction to the injected import. + """ + project = tmp_path / "project" + _write_project(project) + + result = _run_install(project, monkeypatch) + + assert result.exit_code == 0, result.output + assert (project / ".cursor/hooks.json").exists() diff --git a/tests/integration/test_package_target_hook_routing_e2e.py b/tests/integration/test_package_target_hook_routing_e2e.py index 586a41739..036a1f5ab 100644 --- a/tests/integration/test_package_target_hook_routing_e2e.py +++ b/tests/integration/test_package_target_hook_routing_e2e.py @@ -180,6 +180,116 @@ def _consumer( return package +def _cursor_windsurf_consumer( + scenario: _Scenario, + name: str, + dependency: dict[str, object], +) -> LocalPackage: + """Author a cursor+windsurf consumer and create both native marker roots. + + Both roots are pre-created up front: windsurf's merge-hook config is + ``require_dir=True`` (see ``hook_integrator._MERGE_HOOK_TARGETS``), so a + later widen phase that adds windsurf to the package's own targets would + otherwise silently skip the write. + """ + package = scenario.consumers.create( + name, + dependencies=(dependency,), + targets=("cursor", "windsurf"), + ) + (package.root / ".cursor").mkdir() + (package.root / ".windsurf").mkdir() + return package + + +def _repoint_published_package_targets( + scenario: _Scenario, + published: _PublishedPackage, + *, + targets: tuple[str, ...], +) -> _PublishedPackage: + """Advance a published package's own ``targets:`` manifest field. + + Generalizes ``_restrict_published_package_to_claude`` to an arbitrary + target tuple so the same published-package-target-restriction mechanism + (``HookIntegrator.reconcile_package_target_restriction``) can be driven + through both a widen and a narrow transition, not just a one-way + restriction. + """ + manifest_path = published.repository.worktree / "apm.yml" + manifest = load_yaml(manifest_path) + manifest["targets"] = list(targets) + dump_yaml(manifest, manifest_path) + commit = scenario.repositories.commit( + published.repository, + message=f"retarget {published.name} to {list(targets)}", + ) + return _PublishedPackage( + name=published.name, + repository=published.repository, + commit=commit, + remote_url=published.remote_url, + environment=published.environment, + ) + + +_MANUAL_WINDSURF_COMMAND = "echo manual-windsurf" + + +def _write_manual_windsurf_hook(project: LocalPackage) -> None: + """Author one user-authored Windsurf ``PreToolUse`` entry outside APM provenance. + + Windsurf's merged native shape nests handlers under a matcher group + (confirmed against production: ``HookIntegrator.integrate_hooks_for_target`` + writes the same ``{"matcher": ..., "hooks": [...]}`` grouping Claude uses, + not Cursor's flattened entries). Written directly rather than appended to + an existing file because the cursor-only install phase never creates + ``.windsurf/hooks.json``. + """ + path = project.root / ".windsurf" / "hooks.json" + manual_group = { + "matcher": "manual", + "hooks": [{"type": "command", "command": _MANUAL_WINDSURF_COMMAND}], + } + document: dict[str, object] = {"hooks": {"PreToolUse": [manual_group]}} + if path.exists(): + existing = json.loads(path.read_text(encoding="utf-8")) + existing.setdefault("hooks", {}).setdefault("PreToolUse", []).append(manual_group) + document = existing + path.write_text(json.dumps(document, indent=2) + "\n", encoding="utf-8") + + +def _cursor_windsurf_snapshot(project: LocalPackage) -> LifecycleStateSnapshot: + """Capture exact lock, config, and hook ownership state for cursor+windsurf.""" + return LifecycleStateSnapshot.capture( + project.root, + targets=("cursor", "windsurf"), + ) + + +def _windsurf_commands(snapshot: LifecycleStateSnapshot) -> list[str]: + """Return exact commands from Windsurf's native nested matcher groups.""" + state = snapshot.file(".windsurf/hooks.json") + if state.kind == "missing": + return [] + document = json.loads(state.content or b"{}") + commands: list[str] = [] + hooks = document.get("hooks", {}) + assert isinstance(hooks, dict) + for entries in hooks.values(): + assert isinstance(entries, list) + for entry in entries: + assert isinstance(entry, dict) + handlers = entry.get("hooks", [entry]) + assert isinstance(handlers, list) + for handler in handlers: + assert isinstance(handler, dict) + command = handler.get("command") + if isinstance(command, str): + commands.append(command) + return commands + + def _run_success( scenario: _Scenario, project: LocalPackage, @@ -251,6 +361,24 @@ def _expected_sidecar(source: str) -> dict[str, object]: return hooks +def _expected_cursor_sidecar(source: str) -> dict[str, object]: + """Pin native Cursor ownership after a Cursor-only installation.""" + events = {"Stop": "stop", "PreToolUse": "preToolUse", "PostToolUse": "postToolUse"} + return { + events[event]: [ + { + "type": "command", + "command": command, + "timeout": 10, + "_apm_source": source, + "matcher": "*", + **({"loop_limit": None} if event == "Stop" else {}), + } + ] + for event, command in _EVENT_COMMANDS.items() + } + + def _assert_claude_owned_cursor_empty( snapshot: LifecycleStateSnapshot, *, @@ -289,15 +417,11 @@ def _add_manual_cursor_hook(project: LocalPackage) -> None: """Add one user-authored Cursor entry outside APM provenance.""" path = project.root / ".cursor" / "hooks.json" document = json.loads(path.read_text(encoding="utf-8")) - document.setdefault("hooks", {}).setdefault("PreToolUse", []).append( + document.setdefault("hooks", {}).setdefault("preToolUse", []).append( { "matcher": "manual", - "hooks": [ - { - "type": "command", - "command": _MANUAL_CURSOR_COMMAND, - } - ], + "type": "command", + "command": _MANUAL_CURSOR_COMMAND, } ) path.write_text( @@ -307,7 +431,7 @@ def _add_manual_cursor_hook(project: LocalPackage) -> None: def _cursor_commands(snapshot: LifecycleStateSnapshot) -> list[str]: - """Return exact commands from Cursor's nested fixture entries.""" + """Return exact commands from Cursor's native flat entries.""" document = _json(snapshot, ".cursor/hooks.json") commands: list[str] = [] hooks = document.get("hooks", {}) @@ -316,7 +440,7 @@ def _cursor_commands(snapshot: LifecycleStateSnapshot) -> list[str]: assert isinstance(entries, list) for entry in entries: assert isinstance(entry, dict) - handlers = entry.get("hooks", []) + handlers = entry.get("hooks", [entry]) assert isinstance(handlers, list) for handler in handlers: assert isinstance(handler, dict) @@ -372,7 +496,7 @@ def test_package_target_transition_repairs_cursor_and_uninstall_preserves_user_h tmp_path: Path, apm_binary_path: Path, ) -> None: - """Universal-to-Claude update removes only APM-owned Cursor state.""" + """Cursor-to-Claude update removes only APM-owned Cursor state.""" scenario = _new_scenario(tmp_path / "target-transition", apm_binary_path) universal = _publish( scenario, @@ -384,13 +508,13 @@ def test_package_target_transition_repairs_cursor_and_uninstall_preserves_user_h _run_success( scenario, consumer, - _INSTALL_ARGS, + (*_INSTALL_ARGS, "--target", "cursor"), environment=universal.environment, scenario_id="target-transition-universal-install", ) universal_state = _snapshot(consumer) assert set(_cursor_commands(universal_state)) == set(_EVENT_COMMANDS.values()) - assert _json(universal_state, ".cursor/apm-hooks.json") == _expected_sidecar( + assert _json(universal_state, ".cursor/apm-hooks.json") == _expected_cursor_sidecar( f"{_OWNER}/{universal.name}" ) _add_manual_cursor_hook(consumer) @@ -479,7 +603,7 @@ def test_failed_restricted_update_preserves_existing_hook_state( _run_success( scenario, consumer, - _INSTALL_ARGS, + (*_INSTALL_ARGS, "--target", "cursor"), environment=universal.environment, scenario_id="failed-transition-universal-install", ) @@ -506,3 +630,164 @@ def test_failed_restricted_update_preserves_existing_hook_state( output = " ".join((result.stdout + result.stderr).split()) assert "Rejected frontmatter in broken.instructions.md" in output assert _snapshot(consumer) == before + + +def test_consumer_widen_then_drop_cursor_preserves_user_hooks( + tmp_path: Path, apm_binary_path: Path +) -> None: + """An installed CLI retires Cursor, not merely the other target, on contraction.""" + scenario = _new_scenario(tmp_path / "drop-cursor", apm_binary_path) + published = _publish(scenario, "drop-cursor-hooks", targets=("codex", "cursor")) + consumer = scenario.consumers.create( + "codex-consumer", dependencies=(published.dependency,), targets=("codex",) + ) + (consumer.root / ".codex").mkdir() + (consumer.root / ".cursor").mkdir() + retained = None + for phase, targets in ( + ("initial", ["codex"]), + ("widen", ["codex", "cursor"]), + ("narrow", ["codex"]), + ): + manifest_path = consumer.root / "apm.yml" + manifest = load_yaml(manifest_path) + manifest["targets"] = targets + dump_yaml(manifest, manifest_path) + _run_success( + scenario, + consumer, + _INSTALL_ARGS, + environment=published.environment, + scenario_id=f"drop-cursor-{phase}", + ) + state = LifecycleStateSnapshot.capture(consumer.root, targets=("codex", "cursor")) + if phase == "widen": + assert _json(state, ".cursor/apm-hooks.json") == _expected_cursor_sidecar( + f"{_OWNER}/{published.name}" + ) + assert set(_cursor_commands(state)) == set(_EVENT_COMMANDS.values()) + assert all( + "_apm_source" not in entry + for entries in _json(state, ".cursor/hooks.json")["hooks"].values() + for entry in entries + ) + _add_manual_cursor_hook(consumer) + retained = state.file(".codex/hooks.json") + elif phase == "narrow": + assert _json(state, ".cursor/hooks.json") == { + "version": 1, + "hooks": { + "preToolUse": [ + { + "matcher": "manual", + "type": "command", + "command": _MANUAL_CURSOR_COMMAND, + } + ] + }, + } + assert state.file(".cursor/apm-hooks.json").kind == "missing" + assert state.file(".codex/hooks.json") == retained + _run_success( + scenario, + consumer, + ("prune",), + environment=published.environment, + scenario_id="drop-cursor-prune", + ) + assert LifecycleStateSnapshot.capture(consumer.root, targets=("codex", "cursor")).file( + ".cursor/hooks.json" + ) == state.file(".cursor/hooks.json") + + +def test_cursor_windsurf_package_target_widen_then_narrow_retires_owned_windsurf_only( + tmp_path: Path, + apm_binary_path: Path, +) -> None: + """Installed-CLI widen/narrow: Cursor-only -> Cursor+Windsurf -> Cursor-only. + + Exercises the SAME published-package-target-restriction mechanism as + ``test_claude_package_restricts_hooks_across_install_update_and_audit`` + (``HookIntegrator.reconcile_package_target_restriction``) through a real + installed binary, but bidirectionally: a widen that must add the owned + Windsurf hook without touching Cursor, followed by a narrow that must + retire only the owned Windsurf hook, preserving both the surviving + Cursor hook and a pre-existing unowned Windsurf user hook throughout. + """ + scenario = _new_scenario(tmp_path / "cursor-windsurf-transition", apm_binary_path) + cursor_only = _publish(scenario, "cursor-windsurf-hooks", targets=("cursor",)) + consumer = _cursor_windsurf_consumer( + scenario, + "cursor-windsurf-consumer", + cursor_only.dependency, + ) + + # Phase 1: cursor-only install. Windsurf must stay untouched. + _run_success( + scenario, + consumer, + _INSTALL_ARGS, + environment=cursor_only.environment, + scenario_id="cursor-windsurf-cursor-only-install", + ) + phase1 = _cursor_windsurf_snapshot(consumer) + assert set(_cursor_commands(phase1)) == set(_EVENT_COMMANDS.values()) + assert phase1.file(".windsurf/hooks.json").kind == "missing" + + # A pre-existing unowned Windsurf hook must survive both transitions. + _write_manual_windsurf_hook(consumer) + baseline = _cursor_windsurf_snapshot(consumer) + assert _windsurf_commands(baseline) == [_MANUAL_WINDSURF_COMMAND] + + # Phase 2: widen the published package to cursor+windsurf. + widened = _repoint_published_package_targets( + scenario, + cursor_only, + targets=("cursor", "windsurf"), + ) + scenario.consumers.replace_apm_dependencies(consumer, (widened.dependency,)) + _run_success( + scenario, + consumer, + (*_INSTALL_ARGS, "--update"), + environment=widened.environment, + scenario_id="cursor-windsurf-widen-update", + ) + phase2 = _cursor_windsurf_snapshot(consumer) + assert set(_cursor_commands(phase2)) == set(_EVENT_COMMANDS.values()), ( + "widening to add windsurf must not touch the existing cursor hooks" + ) + windsurf_after_widen = set(_windsurf_commands(phase2)) + assert set(_EVENT_COMMANDS.values()) <= windsurf_after_widen, ( + "the package's owned windsurf hook must be installed on widen" + ) + assert _MANUAL_WINDSURF_COMMAND in windsurf_after_widen, ( + "the pre-existing unowned windsurf hook must survive the widen" + ) + + # Phase 3: narrow back to cursor-only. Only the owned windsurf entry + # must be retired; cursor and the unowned windsurf entry must survive. + narrowed = _repoint_published_package_targets( + scenario, + widened, + targets=("cursor",), + ) + scenario.consumers.replace_apm_dependencies(consumer, (narrowed.dependency,)) + _run_success( + scenario, + consumer, + (*_INSTALL_ARGS, "--update"), + environment=narrowed.environment, + scenario_id="cursor-windsurf-narrow-update", + ) + phase3 = _cursor_windsurf_snapshot(consumer) + assert set(_cursor_commands(phase3)) == set(_EVENT_COMMANDS.values()), ( + "narrowing away windsurf must not touch the surviving cursor hooks" + ) + windsurf_after_narrow = set(_windsurf_commands(phase3)) + assert not (set(_EVENT_COMMANDS.values()) & windsurf_after_narrow), ( + "the owned windsurf entries must be retired once windsurf leaves the target set" + ) + assert _MANUAL_WINDSURF_COMMAND in windsurf_after_narrow, ( + "the pre-existing unowned windsurf hook must survive the narrow" + ) diff --git a/tests/integration/test_required_lifecycle_state_machine.py b/tests/integration/test_required_lifecycle_state_machine.py index 5f385e0c0..337f922b4 100644 --- a/tests/integration/test_required_lifecycle_state_machine.py +++ b/tests/integration/test_required_lifecycle_state_machine.py @@ -1390,7 +1390,7 @@ def test_required_target_widen_then_narrow_reconciles_owned_state( ) state_a = LifecycleStateSnapshot.capture(consumer.root, targets=("claude",)) - scenario.consumers.set_targets(consumer, ("claude", "cursor")) + scenario.consumers.set_targets(consumer, ("claude", "windsurf")) _run_success( scenario, consumer, @@ -1400,7 +1400,7 @@ def test_required_target_widen_then_narrow_reconciles_owned_state( ) state_ab = LifecycleStateSnapshot.capture( consumer.root, - targets=("claude", "cursor"), + targets=("claude", "windsurf"), ) scenario.consumers.set_targets(consumer, ("claude",)) @@ -1420,10 +1420,10 @@ def test_required_target_widen_then_narrow_reconciles_owned_state( ) state_final = LifecycleStateSnapshot.capture( consumer.root, - targets=("claude", "cursor"), + targets=("claude", "windsurf"), config_paths=( PurePosixPath(".agents/skills/scope/SKILL.md"), - PurePosixPath(".cursor/rules/scope.mdc"), + PurePosixPath(".windsurf/rules/scope.md"), ), ) _, audit = _audit( @@ -1438,18 +1438,18 @@ def test_required_target_widen_then_narrow_reconciles_owned_state( == state_ab.file(".claude/skills/scope/SKILL.md").content ) assert state_ab.file(".agents/skills/scope/SKILL.md").content == _skill("scope").encode() - assert state_ab.file(".cursor/rules/scope.mdc").kind == "file" - assert state_ab.file(".cursor/hooks.json").kind == "file" - assert state_ab.file(".cursor/apm-hooks.json").kind == "file" + assert state_ab.file(".windsurf/rules/scope.md").kind == "file" + assert state_ab.file(".windsurf/hooks.json").kind == "file" + assert state_ab.file(".windsurf/apm-hooks.json").kind == "file" assert state_final.file(".agents/skills/scope/SKILL.md").kind == "missing" - assert state_final.file(".cursor/rules/scope.mdc").kind == "missing" - assert _hook_commands(consumer.root / ".cursor" / "hooks.json") == [] - assert state_final.file(".cursor/apm-hooks.json").kind == "missing" + assert state_final.file(".windsurf/rules/scope.md").kind == "missing" + assert _hook_commands(consumer.root / ".windsurf" / "hooks.json") == [] + assert state_final.file(".windsurf/apm-hooks.json").kind == "missing" assert ( state_final.file(".claude/skills/scope/SKILL.md").content == state_a.file(".claude/skills/scope/SKILL.md").content ) - assert not any(record.locator.target == "cursor" for record in state_final.deployment_records) + assert not any(record.locator.target == "windsurf" for record in state_final.deployment_records) assert audit["passed"] is True diff --git a/tests/spec_conformance/test_cursor_hook_reqs.py b/tests/spec_conformance/test_cursor_hook_reqs.py new file mode 100644 index 000000000..44d61485c --- /dev/null +++ b/tests/spec_conformance/test_cursor_hook_reqs.py @@ -0,0 +1,277 @@ +"""Cursor-native hook installation conformance -- sec.8.5.9. + +req-tg-016 and req-tg-017 are the spec-citation fold for PR #3149's +already-shipped Cursor-native hook capability: fail-closed conversion +validation and Claude-import-coexistence rejection. These are +silent-deletion detectors that pin the normative phrasing in place; +the actual behavioral proof is the already-existing, already-passing +integration suite named in the editorial note below (not duplicated +here), consistent with this directory's drift-sentinel pattern for +requirements backed by a real shipped implementation rather than a +schema or fixture. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +from tests.spec_conformance._helpers import assert_spec_contains + + +@pytest.mark.req("req-tg-016") +def test_cursor_native_fail_closed_conversion_clause_persists_in_spec() -> None: + """Silent-deletion detector for the req-tg-016 normative clause. + + Behavioral proof (not duplicated here): + tests/unit/integration/test_cursor_hook_native_contract.py:: + test_unrepresentable_hooks_fail_without_native_or_script_writes, + ::test_cursor_install_emits_native_events_and_flat_handlers. + """ + assert_spec_contains( + "MUST fail closed when converting a source-declared hook\n" + "configuration into that Cursor-native format", + "the implementation MUST NOT write the Cursor-native hook\n" + "artifact (zero bytes, no partial file)", + "MUST emit an actionable diagnostic\nnaming the unrecognized value(s)", + ) + + +@pytest.mark.req("req-tg-017") +def test_cursor_claude_import_overlap_rejection_clause_persists_in_spec() -> None: + """Silent-deletion detector for the req-tg-017 normative clause. + + Behavioral proof (not duplicated here): + tests/unit/integration/test_cursor_hook_native_contract.py:: + test_import_overlap_refused_in_either_install_order, + ::test_import_locations_preserved_and_overlap_rejected, + ::test_unapproved_hooks_do_not_enter_cursor_preflight; + tests/integration/test_package_target_hook_routing_e2e.py:: + test_package_target_transition_repairs_cursor_and_uninstall_preserves_user_hook, + ::test_failed_restricted_update_preserves_existing_hook_state; + tests/integration/test_cursor_hook_lifecycle.py:: + test_cursor_installed_cli_contract. + """ + assert_spec_contains( + "for\nboth install orders (Claude-import-first and Cursor-native-first)", + "MUST reject the write with an actionable\n" + "diagnostic rather than merge, redirect, or broaden either side's accepted\n" + "input", + "MUST NOT alter the Claude import's own settings", + ) + + +def _write_json(path: Path, payload: object) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps(payload, indent=2) + "\n", encoding="utf-8") + + +def _package(tmp_path: Path, hooks: dict[str, object]): + from apm_cli.models.apm_package import APMPackage, PackageInfo + + root = tmp_path / "req-tg-fixture" + _write_json(root / ".apm/hooks/gate.json", {"hooks": hooks}) + script = root / ".apm/hooks/scripts/gate.py" + script.parent.mkdir(parents=True) + script.write_text("print('{}')\n", encoding="utf-8") + return PackageInfo( + package=APMPackage(name="req-tg-fixture", version="1.0.0"), + install_path=root, + ) + + +@pytest.mark.req("req-tg-016") +def test_cursor_native_fail_closed_conversion_is_real_not_prose(tmp_path: Path) -> None: + """Real fail-closed behavior proof for req-tg-016, with a mutation control. + + The positive case proves an out-of-vocabulary source event is rejected + before any native artifact is written. The mutation control removes the + fail-closed guard (by widening the accepted event vocabulary, the same + class of regression a reordered/removed validation check would cause) + and proves the write would otherwise silently succeed -- demonstrating + the positive assertion is load-bearing, not an accidental pass. + """ + from apm_cli.hook_contract import HookContractError + from apm_cli.integration.hook_integrator import HookIntegrator + from apm_cli.integration.targets import KNOWN_TARGETS + + package = _package(tmp_path, {"Notification": [{"hooks": [{"command": "echo bad"}]}]}) + project = tmp_path / "project" + (project / ".cursor").mkdir(parents=True) + + with pytest.raises(HookContractError, match="unsupported Cursor event"): + HookIntegrator().integrate_hooks_for_target(KNOWN_TARGETS["cursor"], package, project) + + assert list((project / ".cursor").iterdir()) == [], ( + "a rejected conversion MUST NOT write the Cursor-native hook artifact" + ) + + from apm_cli.integration import hook_integrator as integrator_mod + + cursor_event_map = integrator_mod._HOOK_EVENT_MAP["cursor"] + assert "Notification" not in cursor_event_map + + cursor_event_map["Notification"] = "preToolUse" + try: + widened_project = tmp_path / "project-widened" + (widened_project / ".cursor").mkdir(parents=True) + package_widened = _package( + tmp_path / "widened-src", {"Notification": [{"hooks": [{"command": "echo bad"}]}]} + ) + HookIntegrator().integrate_hooks_for_target( + KNOWN_TARGETS["cursor"], package_widened, widened_project + ) + assert (widened_project / ".cursor/hooks.json").exists(), ( + "mutation control: widening the accepted event vocabulary (the same " + "class of regression a removed/reordered fail-closed check would " + "cause) MUST let the same out-of-vocabulary source event through, " + "proving the guard above is the thing actually stopping it" + ) + finally: + del cursor_event_map["Notification"] + + assert_spec_contains( + "MUST fail closed when converting a source-declared hook", + "the implementation MUST NOT write the Cursor-native hook\n" + "artifact (zero bytes, no partial file)", + ) + + +@pytest.mark.req("req-tg-017") +def test_cursor_claude_overlap_predicate_is_kind_aware_not_event_only(tmp_path: Path) -> None: + """Real behavior proof for req-tg-017's corrected narrower predicate. + + Positive case: the same event with the same literal text but a + different handler *kind* (``command`` vs ``prompt``) is NOT an + overlap under the corrected (event, kind, content) predicate. + Mutation control: patching the predicate back to the stale + event-and-content-only (kind-blind) comparison this spec clause used + to describe MUST turn that same scenario into a rejection, proving + the positive case depends on kind-awareness rather than passing by + coincidence. + """ + from apm_cli.hook_contract import HookContractError + from apm_cli.integration.hook_integrator import HookIntegrator + from apm_cli.integration.targets import KNOWN_TARGETS + + package = _package( + tmp_path, {"PreToolUse": [{"hooks": [{"type": "command", "command": "echo shared"}]}]} + ) + project = tmp_path / "project" + (project / ".cursor").mkdir(parents=True) + claude = project / ".claude/settings.json" + _write_json( + claude, + { + "hooks": { + "PreToolUse": [ + {"matcher": "Bash", "hooks": [{"type": "prompt", "prompt": "echo shared"}]} + ] + } + }, + ) + before = claude.read_bytes() + + result = HookIntegrator().integrate_hooks_for_target(KNOWN_TARGETS["cursor"], package, project) + + assert result.files_integrated == 1 + assert claude.read_bytes() == before + assert (project / ".cursor/hooks.json").exists() + + import apm_cli.integration.hook_cursor_preflight as preflight_mod + + real_action_keys = preflight_mod._action_keys + + def _kind_blind_action_keys(document, event_map, roots, owners, *, owned=False): + keys = real_action_keys(document, event_map, roots, owners, owned=owned) + return {(event, "command", content) for event, _kind, content in keys} + + preflight_mod._action_keys = _kind_blind_action_keys + try: + package2 = _package( + tmp_path / "kind-blind-src", + {"PreToolUse": [{"hooks": [{"type": "command", "command": "echo shared"}]}]}, + ) + project2 = tmp_path / "project-kind-blind" + (project2 / ".cursor").mkdir(parents=True) + _write_json( + project2 / ".claude/settings.json", + { + "hooks": { + "PreToolUse": [ + { + "matcher": "Bash", + "hooks": [{"type": "prompt", "prompt": "echo shared"}], + } + ] + } + }, + ) + with pytest.raises(HookContractError, match="Claude import"): + HookIntegrator().integrate_hooks_for_target(KNOWN_TARGETS["cursor"], package2, project2) + finally: + preflight_mod._action_keys = real_action_keys + + # Needles below were reflowed for req-tg-017's F2/F6 clarification fold + # (alias-normalization scope + both-install-order symmetry wording); the + # substring content is unchanged, only the line wrap and an added + # "identifier" qualifier after "event" moved. + assert_spec_contains( + "is a non-empty intersection, after\nalias normalization, between the set of (event " + "identifier, handler kind,\nhandler content) tuples", + "Two entries for the\nsame event identifier but a different handler kind or different " + "handler\ncontent are not an overlap", + ) + + +@pytest.mark.req("req-tg-017") +def test_cursor_claude_import_rejects_real_default_same_event_kind_content_overlap( + tmp_path: Path, +) -> None: + """Real default-predicate proof for req-tg-017's rejection branch. + + The test above (``test_cursor_claude_overlap_predicate_is_kind_aware_not_ + event_only``) only reaches a rejection by deliberately patching + ``_action_keys`` back to a kind-blind mutant; that proves the predicate + is load-bearing, but not that the unpatched, as-shipped predicate + actually rejects a genuine overlap. This case uses the real nested + Claude handler shape (matching ``_nested`` in the native-contract unit + test) with the SAME event, SAME handler kind ("command"), and SAME + content as the Cursor source hook -- an unambiguous default-production + overlap -- and asserts rejection with zero native writes and a + byte-preserved Claude import, using the unpatched predicate end to end. + """ + package = _package( + tmp_path, {"PreToolUse": [{"hooks": [{"type": "command", "command": "echo shared"}]}]} + ) + project = tmp_path / "project" + (project / ".cursor").mkdir(parents=True) + claude = project / ".claude/settings.json" + _write_json( + claude, + { + "hooks": { + "PreToolUse": [ + { + "matcher": "Bash", + "hooks": [{"type": "command", "command": "echo shared"}], + } + ] + } + }, + ) + before = claude.read_bytes() + + from apm_cli.hook_contract import HookContractError + from apm_cli.integration.hook_integrator import HookIntegrator + from apm_cli.integration.targets import KNOWN_TARGETS + + with pytest.raises(HookContractError, match="Claude import"): + HookIntegrator().integrate_hooks_for_target(KNOWN_TARGETS["cursor"], package, project) + + assert claude.read_bytes() == before + assert list((project / ".cursor").iterdir()) == [], ( + "a rejected Claude-import overlap MUST NOT write the Cursor-native hook artifact" + ) diff --git a/tests/spec_conformance/test_lockfile_reqs.py b/tests/spec_conformance/test_lockfile_reqs.py index 6391c493a..2986f76c8 100644 --- a/tests/spec_conformance/test_lockfile_reqs.py +++ b/tests/spec_conformance/test_lockfile_reqs.py @@ -886,3 +886,77 @@ def test_dropped_target_merge_hook_state_reconciled_fail_safe(tmp_path): "target while its ownership record remains", "MUST leave that document or record unmodified and\nemit an actionable diagnostic", ) + + +@pytest.mark.req("req-lk-021") +def test_dropped_cursor_target_merge_hook_state_reconciled_fail_safe(tmp_path): + """req-lk-021's preserve/remove decision also covers Cursor's flat + native hook format, where ownership is attributed inline via + ``_apm_source`` on each entry rather than via a separate sidecar + file: a dropped Cursor target's consumer-owned entries are removed, + an entry without the consumer's own ownership attribution survives + even in the dropped target's own file, and a retained target (here + Claude's merge-based settings) is preserved untouched.""" + import json + + from apm_cli.integration.hook_integrator import HookIntegrator + + cursor_dir = tmp_path / ".cursor" + cursor_dir.mkdir(parents=True) + (cursor_dir / "hooks.json").write_text( + json.dumps( + { + "version": 1, + "hooks": { + "preToolUse": [ + { + "command": "owned", + "_apm_source": "req-lk-021-fixture", + }, + {"command": "user-authored"}, + ] + }, + } + ), + encoding="utf-8", + ) + + claude_dir = tmp_path / ".claude" + claude_dir.mkdir(parents=True) + (claude_dir / "settings.json").write_text( + json.dumps({"hooks": {"PreToolUse": [{"matcher": "Bash", "hooks": []}]}}), + encoding="utf-8", + ) + (claude_dir / "apm-hooks.json").write_text( + json.dumps({"PreToolUse": [{"matcher": "Bash", "_apm_source": "req-lk-021-fixture"}]}), + encoding="utf-8", + ) + claude_snapshot = (claude_dir / "settings.json").read_text(encoding="utf-8") + + stats = HookIntegrator().reconcile_dropped_targets(tmp_path, ["cursor"]) + + assert stats["errors"] == 0 + cursor_native = json.loads((cursor_dir / "hooks.json").read_text(encoding="utf-8")) + cursor_entries = cursor_native.get("hooks", {}).get("preToolUse", []) + assert not any(e.get("command") == "owned" for e in cursor_entries), ( + "consumer-owned entry for the dropped Cursor target MUST be removed" + ) + assert any(e.get("command") == "user-authored" for e in cursor_entries), ( + "entry without consumer ownership attribution MUST be preserved" + ) + assert not (cursor_dir / "apm-hooks.json").exists(), ( + "Cursor has no ownership sidecar to begin with; reconciliation MUST NOT invent one" + ) + assert claude_snapshot == (claude_dir / "settings.json").read_text(encoding="utf-8"), ( + "a target still attributable to the declared set MUST be preserved untouched" + ) + assert (claude_dir / "apm-hooks.json").exists(), "retained target's ownership record survives" + + assert_spec_contains( + "MUST apply the same preserve-or-remove decision", + "MUST remove only the consumer-owned entries", + "It MUST preserve\nevery entry that does not carry the consumer's own ownership", + "the merge-based hook configuration document is already absent for a\n" + "target while its ownership record remains", + "MUST leave that document or record unmodified and\nemit an actionable diagnostic", + ) diff --git a/tests/unit/integration/test_cursor_hook_native_contract.py b/tests/unit/integration/test_cursor_hook_native_contract.py new file mode 100644 index 000000000..d17225dd0 --- /dev/null +++ b/tests/unit/integration/test_cursor_hook_native_contract.py @@ -0,0 +1,996 @@ +"""Cursor native output and third-party import regression contracts.""" + +from __future__ import annotations + +import json +from dataclasses import replace +from pathlib import Path +from typing import Any +from unittest.mock import MagicMock, patch + +import pytest + +from apm_cli.core.scope import InstallScope +from apm_cli.hook_contract import HookContractError +from apm_cli.install.deployable_source_plan import DeployableSourcePlan +from apm_cli.install.services import IntegratorBundle, integrate_package_primitives +from apm_cli.integration import hook_integrator +from apm_cli.integration.hook_integrator import HookIntegrator, native_hook_config +from apm_cli.integration.hook_native_formats import ( + _to_cursor_hook_entries, + inspect_native_hooks, + validate_cursor_config, +) +from apm_cli.integration.skill_integrator import SkillIntegrator +from apm_cli.integration.targets import KNOWN_TARGETS +from apm_cli.models.apm_package import APMPackage, PackageInfo +from apm_cli.utils.diagnostics import DiagnosticCollector + +pytestmark = pytest.mark.component + + +def _write_json(path: Path, payload: object) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps(payload, indent=2) + "\n", encoding="utf-8") + + +def _package(tmp_path: Path, hooks: dict[str, Any]) -> PackageInfo: + root = tmp_path / "cursor-gate" + _write_json(root / ".apm/hooks/gate.json", {"hooks": hooks}) + script = root / ".apm/hooks/scripts/gate.py" + script.parent.mkdir(parents=True) + script.write_text("print('{}')\n", encoding="utf-8") + return PackageInfo( + package=APMPackage(name="cursor-gate", version="1.0.0"), + install_path=root, + ) + + +def _nested(command: str, **group: Any) -> dict[str, Any]: + return {**group, "hooks": [{"type": "command", "command": command, "timeout": 10}]} + + +@pytest.mark.parametrize( + ("field", "value", "message"), + [ + ("matcher", 12, "Cursor matcher must be a string"), + ("timeout", False, "Cursor timeout must be a finite positive number of seconds"), + ("timeout", -1, "Cursor timeout must be a finite positive number of seconds"), + ("timeout", float("inf"), "Cursor timeout must be a finite positive number of seconds"), + ("failClosed", "true", "Cursor failClosed must be a boolean"), + ], +) +def test_cursor_source_and_native_field_rejections_agree( + field: str, value: object, message: str +) -> None: + """Both consumers exercise the same field predicates across normalization.""" + entry = {"command": "echo check", field: value} + with pytest.raises(HookContractError) as source_error: + _to_cursor_hook_entries([entry], "preToolUse") + with pytest.raises(HookContractError) as native_error: + validate_cursor_config({"version": 1, "hooks": {"preToolUse": [entry]}}) + assert str(source_error.value) == str(native_error.value) == message + + +@pytest.mark.parametrize( + ("hooks", "message"), + [ + ({"PreToolUse": [_nested("echo check", matcher="Bash | Read")]}, "contain whitespace"), + ({"preToolUse": [{"command": "echo check", "mysteryField": True}]}, "mysteryField"), + ( + {"PreToolUse": [{**_nested("echo check", matcher="Bash"), "mysteryGroupField": True}]}, + "mysteryGroupField", + ), + ], +) +def test_cursor_diagnostic_identifies_invalid_input_before_writes( + tmp_path: Path, hooks: dict[str, Any], message: str +) -> None: + package = _package(tmp_path, hooks) + project = tmp_path / "project" + (project / ".cursor").mkdir(parents=True) + with pytest.raises(HookContractError, match=message): + HookIntegrator().integrate_hooks_for_target(KNOWN_TARGETS["cursor"], package, project) + assert not list(project.rglob("*.json")) + + +def test_cursor_install_emits_native_events_and_flat_handlers(tmp_path: Path) -> None: + """Pin native JSON independently of the production event/handler tables.""" + command = 'python "${PLUGIN_ROOT}/.apm/hooks/scripts/gate.py"' + package = _package( + tmp_path, + { + "PreToolUse": [_nested(command, matcher="Bash")], + "PostToolUse": [_nested(command, matcher="Edit|Write")], + "UserPromptSubmit": [_nested(command)], + "PreCompact": [_nested(command)], + "Stop": [_nested(command)], + }, + ) + project = tmp_path / "project" + (project / ".cursor").mkdir(parents=True) + + result = HookIntegrator().integrate_hooks_for_target(KNOWN_TARGETS["cursor"], package, project) + + native = { + "type": "command", + "command": 'python ".cursor/hooks/cursor-gate/.apm/hooks/scripts/gate.py"', + "timeout": 10, + } + expected = { + "version": 1, + "hooks": { + "preToolUse": [{**native, "matcher": "Shell"}], + "postToolUse": [{**native, "matcher": "Write"}], + "beforeSubmitPrompt": [native], + "preCompact": [native], + "stop": [{**native, "loop_limit": None}], + }, + } + assert json.loads((project / ".cursor/hooks.json").read_text()) == expected + assert result.files_integrated == 1 + assert (project / ".cursor/hooks/cursor-gate/.apm/hooks/scripts/gate.py").is_file() + assert json.loads(result.display_payloads[0]["rendered_json"])["hooks"] == expected["hooks"] + + +def test_cursor_refusal_precedes_all_hook_bundle_writes(tmp_path: Path) -> None: + package = _package(tmp_path, {"preToolUse": [{"command": "echo ok"}]}) + hooks = package.install_path / ".apm/hooks" + _write_json( + hooks / "a-valid.json", + {"hooks": {"preToolUse": [_nested('python "${PLUGIN_ROOT}/.apm/hooks/scripts/gate.py"')]}}, + ) + _write_json(hooks / "z-unsupported.json", {"hooks": {"Notification": [_nested("echo bad")]}}) + project = tmp_path / "project" + user = {"version": 1, "hooks": {"afterFileEdit": [{"command": "echo user"}]}} + path = project / ".cursor/hooks.json" + _write_json(path, user) + before = path.read_bytes() + + with pytest.raises(HookContractError, match="unsupported Cursor event"): + HookIntegrator().integrate_hooks_for_target(KNOWN_TARGETS["cursor"], package, project) + + assert path.read_bytes() == before + assert sorted(p.relative_to(project).as_posix() for p in project.rglob("*") if p.is_file()) == [ + ".cursor/hooks.json" + ] + + +@pytest.mark.parametrize("first", ["cursor", "claude"]) +def test_import_overlap_refused_in_either_install_order(tmp_path: Path, first: str) -> None: + package = _package(tmp_path, {"PreToolUse": [_nested("echo shared", matcher="Bash")]}) + project = tmp_path / "project" + (project / ".cursor").mkdir(parents=True) + integrator = HookIntegrator() + integrator.integrate_hooks_for_target(KNOWN_TARGETS[first], package, project) + before = {p.relative_to(project): p.read_bytes() for p in project.rglob("*") if p.is_file()} + second = "claude" if first == "cursor" else "cursor" + + with pytest.raises(HookContractError, match="Claude import"): + integrator.integrate_hooks_for_target(KNOWN_TARGETS[second], package, project) + + after = {p.relative_to(project): p.read_bytes() for p in project.rglob("*") if p.is_file()} + assert after == before + + +def test_planned_overlap_refused_before_either_target_is_written(tmp_path: Path) -> None: + package = _package(tmp_path, {"PreToolUse": [_nested("echo shared", matcher="Bash")]}) + project = tmp_path / "project" + (project / ".cursor").mkdir(parents=True) + targets = [KNOWN_TARGETS[name] for name in ("claude", "cursor")] + plan = DeployableSourcePlan.create( + package, + targets, + skill_subset=None, + hooks_approved=True, + canvas_approved=False, + skip_bin=True, + ) + + with pytest.raises(HookContractError, match="Claude import"): + HookIntegrator().integrate_hooks_for_target(targets[0], package, project, source_plan=plan) + + assert not list(project.rglob("*.json")) + + +@pytest.mark.parametrize( + ("source", "native"), + [ + ("PreToolUse", "preToolUse"), + ("PostToolUse", "postToolUse"), + ("UserPromptSubmit", "beforeSubmitPrompt"), + ("Stop", "stop"), + ("SubagentStop", "subagentStop"), + ("SessionStart", "sessionStart"), + ("SessionEnd", "sessionEnd"), + ("PreCompact", "preCompact"), + ], +) +def test_documented_cursor_import_event_mappings(tmp_path: Path, source: str, native: str) -> None: + package = _package(tmp_path, {source: [_nested("echo check")]}) + project = tmp_path / "project" + (project / ".cursor").mkdir(parents=True) + + HookIntegrator().integrate_hooks_for_target(KNOWN_TARGETS["cursor"], package, project) + + data = json.loads((project / ".cursor/hooks.json").read_text()) + assert list(data["hooks"]) == [native] + entry = data["hooks"][native][0] + assert entry["command"] == "echo check" + assert "hooks" not in entry + if native in ("stop", "subagentStop"): + assert entry["loop_limit"] is None + + +@pytest.mark.parametrize( + ("event", "entry", "message"), + [ + ("Notification", _nested("echo check"), "unsupported Cursor event"), + ("PermissionRequest", _nested("echo check"), "unsupported Cursor event"), + ("userPromptSubmitted", _nested("echo check"), "unsupported Cursor event"), + ("agentStop", _nested("echo check"), "unsupported Cursor event"), + ("PreToolUse", _nested("echo check", matcher="mcp__ide__.*"), "cannot preserve"), + ("PreToolUse", _nested("echo check", matcher="Glob|Bash"), "cannot preserve"), + ("PreToolUse", _nested("echo check", matcher="^Bash$"), "cannot preserve"), + ("PreToolUse", _nested("echo check", matcher="Write"), "both Claude Edit and Write"), + ("PreToolUse", _nested("echo check", matcher="Edit"), "both Claude Edit and Write"), + ("SessionStart", _nested("echo check", matcher="startup"), "no verified Cursor equivalent"), + ("preToolUse", {"bash": "echo check"}, "platform-specific"), + ( + "preToolUse", + {"command": "echo check", "linux": "echo posix"}, + "unsupported Cursor handler", + ), + ("preToolUse", {"command": "echo check", "async": True}, "unsupported Cursor handler"), + ("preToolUse", {"type": "agent", "prompt": "verify"}, "only command and prompt"), + ("preToolUse", {"type": {}, "command": "echo check"}, "only command and prompt"), + ("preToolUse", {"command": "echo check", "matcher": None}, "matcher must be a string"), + ("preToolUse", {"command": "echo check", "timeout": None}, "finite positive"), + ("preToolUse", {"command": "echo check", "timeout": True}, "finite positive"), + ("preToolUse", {"command": "echo check", "timeout": float("inf")}, "finite positive"), + ( + "preToolUse", + {"command": "echo check", "timeout": 2, "timeoutSec": 3}, + "only one timeout", + ), + ("preToolUse", {"command": "echo check", "failClosed": "true"}, "must be a boolean"), + ], +) +def test_unrepresentable_hooks_fail_without_native_or_script_writes( + tmp_path: Path, event: str, entry: dict[str, Any], message: str +) -> None: + package = _package(tmp_path, {event: [entry]}) + project = tmp_path / "project" + (project / ".cursor").mkdir(parents=True) + + with pytest.raises(HookContractError, match=message): + HookIntegrator().integrate_hooks_for_target(KNOWN_TARGETS["cursor"], package, project) + + assert list((project / ".cursor").iterdir()) == [] + + +def test_native_prompt_matcher_and_restrictions_are_preserved(tmp_path: Path) -> None: + native = { + "type": "prompt", + "prompt": "Allow only safe $ARGUMENTS", + "model": "test-model", + "timeout": 10, + "failClosed": True, + "matcher": "curl|wget", + } + package = _package(tmp_path, {"beforeShellExecution": [native]}) + project = tmp_path / "project" + (project / ".cursor").mkdir(parents=True) + + HookIntegrator().integrate_hooks_for_target(KNOWN_TARGETS["cursor"], package, project) + + assert json.loads((project / ".cursor/hooks.json").read_text()) == { + "version": 1, + "hooks": {"beforeShellExecution": [native]}, + } + contract = native_hook_config("cursor") + assert contract is not None + inspected = inspect_native_hooks( + json.loads((project / ".cursor/hooks.json").read_text()), + "cursor_hooks", + prompt_types=contract.prompt_handler_types, + nested_handlers=contract.nested_handlers, + ) + assert [entry.prompt for entry in inspected] == [native["prompt"]] + assert all(entry.error is None for entry in inspected) + + +def test_cursor_install_reinstall_and_remove_preserve_user_hooks(tmp_path: Path) -> None: + package = _package(tmp_path, {"PreToolUse": [_nested("echo installed")]}) + project = tmp_path / "project" + path = project / ".cursor/hooks.json" + user = {"version": 1, "hooks": {"afterFileEdit": [{"command": "echo user"}]}} + _write_json(path, user) + integrator = HookIntegrator() + target = KNOWN_TARGETS["cursor"] + integrator.integrate_hooks_for_target(target, package, project) + first = path.read_bytes() + + integrator.integrate_hooks_for_target(target, package, project) + + assert path.read_bytes() == first + stats = integrator.reconcile_package_target_restriction(package, project, [target]) + assert stats["errors"] == 0 + assert json.loads(path.read_text()) == user + assert not path.with_name("apm-hooks.json").exists() + + +def test_cursor_windsurf_widen_then_narrow_retires_owned_windsurf_hooks_only( + tmp_path: Path, +) -> None: + """Cursor-only -> Cursor+Windsurf -> Cursor-only lifecycle (#3129 general- + delta gap): widening a dependency's targets to also deploy to Windsurf, + then narrowing back to Cursor-only, must retire ONLY this package's owned + Windsurf entry. The Cursor-native hook must stay untouched throughout, and + a pre-existing unowned/user Windsurf hook must survive both the widen and + the narrow unchanged. + """ + package = _package(tmp_path, {"PreToolUse": [_nested("echo installed")]}) + project = tmp_path / "project" + (project / ".cursor").mkdir(parents=True) + integrator = HookIntegrator() + cursor_target = KNOWN_TARGETS["cursor"] + windsurf_target = KNOWN_TARGETS["windsurf"] + + # Phase 1: Cursor-only. + integrator.integrate_hooks_for_target(cursor_target, package, project) + cursor_path = project / ".cursor/hooks.json" + cursor_snapshot = cursor_path.read_bytes() + + # A pre-existing, unowned user hook already lives in Windsurf's config + # before this package ever touches it. + windsurf_path = project / ".windsurf/hooks.json" + user_windsurf = { + "version": 1, + "hooks": {"afterFileEdit": [{"command": "echo user-windsurf"}]}, + } + _write_json(windsurf_path, user_windsurf) + + # Phase 2: widen -- add Windsurf alongside the existing Cursor hook. + integrator.integrate_hooks_for_target(windsurf_target, package, project) + assert cursor_path.read_bytes() == cursor_snapshot, ( + "widening to add windsurf must not touch the existing cursor hook" + ) + windsurf_after_widen = json.loads(windsurf_path.read_text()) + assert windsurf_after_widen["hooks"]["PreToolUse"], ( + "the package's owned windsurf hook must be installed on widen" + ) + assert ( + windsurf_after_widen["hooks"]["afterFileEdit"] == user_windsurf["hooks"]["afterFileEdit"] + ), "the pre-existing unowned windsurf hook must survive the widen" + + # Phase 3: narrow back -- retire only the owned windsurf entry. + stats = integrator.reconcile_package_target_restriction(package, project, [windsurf_target]) + assert stats["errors"] == 0 + + assert cursor_path.read_bytes() == cursor_snapshot, ( + "narrowing away windsurf must not touch the surviving cursor hook" + ) + windsurf_after_narrow = json.loads(windsurf_path.read_text()) + assert "PreToolUse" not in windsurf_after_narrow.get("hooks", {}), ( + "the owned windsurf entry must be retired once windsurf leaves the target set" + ) + assert ( + windsurf_after_narrow["hooks"]["afterFileEdit"] == user_windsurf["hooks"]["afterFileEdit"] + ), "the pre-existing unowned windsurf hook must survive the narrow" + + +def test_reinstall_migrates_only_owned_legacy_events(tmp_path: Path) -> None: + package = _package(tmp_path, {"PreToolUse": [_nested("echo installed")]}) + project = tmp_path / "project" + legacy = _nested("echo installed") + _write_json( + project / ".cursor/hooks.json", + { + "version": 1, + "hooks": {"PreToolUse": [legacy], "afterFileEdit": [{"command": "echo user"}]}, + }, + ) + _write_json( + project / ".cursor/apm-hooks.json", + { + "PreToolUse": [{**legacy, "_apm_source": "cursor-gate"}], + }, + ) + + HookIntegrator().integrate_hooks_for_target(KNOWN_TARGETS["cursor"], package, project) + + data = json.loads((project / ".cursor/hooks.json").read_text()) + assert data["hooks"] == { + "afterFileEdit": [{"command": "echo user"}], + "preToolUse": [{"type": "command", "command": "echo installed", "timeout": 10}], + } + assert set(json.loads((project / ".cursor/apm-hooks.json").read_text())) == {"preToolUse"} + + +@pytest.mark.parametrize( + "payload", + [ + "{broken", + "[]", + '{"version":2,"hooks":{}}', + '{"version":1,"hooks":{"PreToolUse":[{"command":"echo user"}]}}', + ], +) +def test_invalid_user_config_is_not_replaced(tmp_path: Path, payload: str) -> None: + package = _package(tmp_path, {"preToolUse": [{"command": "echo installed"}]}) + project = tmp_path / "project" + path = project / ".cursor/hooks.json" + path.parent.mkdir(parents=True) + path.write_text(payload) + + with pytest.raises(HookContractError): + HookIntegrator().integrate_hooks_for_target(KNOWN_TARGETS["cursor"], package, project) + + assert path.read_text() == payload + assert not path.with_name("apm-hooks.json").exists() + + +@pytest.mark.parametrize("location", ["project", "local", "user"]) +def test_import_locations_preserved_and_overlap_rejected( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, location: str +) -> None: + package = _package(tmp_path, {"PreToolUse": [_nested("echo shared")]}) + project, home = tmp_path / "project", tmp_path / "home" + monkeypatch.setenv("HOME", str(home)) + (project / ".cursor").mkdir(parents=True) + paths = { + "project": project / ".claude/settings.json", + "local": project / ".claude/settings.local.json", + "user": home / ".claude/settings.json", + } + _write_json(paths[location], {"hooks": {"PreToolUse": [_nested("echo shared")]}}) + before = paths[location].read_bytes() + + with pytest.raises(HookContractError, match="Claude import"): + HookIntegrator().integrate_hooks_for_target(KNOWN_TARGETS["cursor"], package, project) + + assert paths[location].read_bytes() == before + assert not (project / ".cursor/hooks.json").exists() + + +def test_unrelated_claude_user_hooks_do_not_block_cursor(tmp_path: Path) -> None: + package = _package(tmp_path, {"PreToolUse": [_nested("echo installed")]}) + project = tmp_path / "project" + (project / ".cursor").mkdir(parents=True) + claude = project / ".claude/settings.json" + _write_json(claude, {"hooks": {"PreToolUse": [_nested("echo unrelated")]}}) + before = claude.read_bytes() + + result = HookIntegrator().integrate_hooks_for_target(KNOWN_TARGETS["cursor"], package, project) + + assert result.files_integrated == 1 + assert claude.read_bytes() == before + + +@pytest.mark.parametrize("names", [("claude", "cursor"), ("cursor", "claude")]) +@pytest.mark.parametrize("cursor_exists", [False, True]) +def test_install_service_preflight_prevents_partial_multi_target_deployment( + tmp_path: Path, names: tuple[str, str], cursor_exists: bool +) -> None: + package = _package(tmp_path, {"PreToolUse": [_nested("echo shared")]}) + project = tmp_path / "project" + project.mkdir() + if cursor_exists: + (project / ".cursor").mkdir() + sibling = MagicMock() + sibling.preflight_instructions_for_targets.side_effect = AssertionError( + "Hook preflight must reject before reaching sibling primitives" + ) + bundle = IntegratorBundle( + prompt=sibling, + agent=sibling, + skill=sibling, + instruction=sibling, + command=sibling, + hook=HookIntegrator(), + ) + + with pytest.raises(HookContractError, match="Claude import"): + integrate_package_primitives( + package, + project, + targets=[KNOWN_TARGETS[name] for name in names], + integrators=bundle, + diagnostics=DiagnosticCollector(), + force=False, + managed_files=None, + ) + + assert not list(project.rglob("*.json")) + sibling.integrate_package_skill.assert_not_called() + sibling.preflight_instructions_for_targets.assert_not_called() + + +def test_unapproved_hooks_do_not_enter_cursor_preflight(tmp_path: Path) -> None: + package = _package(tmp_path, {"Notification": [_nested("echo never")]}) + project = tmp_path / "project" + (project / ".cursor").mkdir(parents=True) + hook = MagicMock() + target = KNOWN_TARGETS["cursor"] + bundle = IntegratorBundle( + prompt=None, agent=None, skill=SkillIntegrator(), instruction=None, command=None, hook=hook + ) + + integrate_package_primitives( + package, + project, + targets=[replace(target, primitives={"hooks": target.primitives["hooks"]})], + integrators=bundle, + diagnostics=DiagnosticCollector(), + allow_executables={}, + force=False, + managed_files=None, + ) + + hook.preflight_hooks_for_targets.assert_not_called() + hook.integrate_hooks_for_target.assert_not_called() + assert not list(project.rglob("*.json")) + + +@pytest.mark.parametrize("first", ["claude", "cursor"]) +@pytest.mark.parametrize("user_scope", [False, True], ids=["project", "home"]) +def test_explicit_target_contraction_can_choose_one_import_route( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, first: str, user_scope: bool +) -> None: + package = _package(tmp_path, {"PreToolUse": [_nested("echo shared")]}) + home = tmp_path / "home" + monkeypatch.setenv("HOME", str(home)) + project = home if user_scope else tmp_path / "project" + (project / ".cursor").mkdir(parents=True) + hook = HookIntegrator() + hook.integrate_hooks_for_target(KNOWN_TARGETS[first], package, project, user_scope=user_scope) + second = "cursor" if first == "claude" else "claude" + package.package = APMPackage(name="cursor-gate", version="1.0.0", targets=[second]) + targets = [ + replace(target, primitives={"hooks": target.primitives["hooks"]}) + for target in (KNOWN_TARGETS["claude"], KNOWN_TARGETS["cursor"]) + ] + + integrate_package_primitives( + package, + project, + targets=targets, + force=False, + managed_files=None, + integrators=IntegratorBundle( + prompt=None, + agent=None, + skill=SkillIntegrator(), + instruction=None, + command=None, + hook=hook, + ), + diagnostics=DiagnosticCollector(), + scope=InstallScope.USER if user_scope else InstallScope.PROJECT, + ) + + first_path = project / f".{first}" / ("settings.json" if first == "claude" else "hooks.json") + second_path = project / f".{second}" / ("settings.json" if second == "claude" else "hooks.json") + assert not json.loads(first_path.read_text()).get("hooks") + assert json.loads(second_path.read_text())["hooks"] + assert not first_path.with_name("apm-hooks.json").exists() + + +@pytest.mark.parametrize("field", ["disableAllHooks", "enabled"]) +def test_cursor_does_not_discard_source_level_settings(tmp_path: Path, field: str) -> None: + package = _package(tmp_path, {"preToolUse": [{"command": "echo gated"}]}) + _write_json( + package.install_path / ".apm/hooks/gate.json", + {field: False, "hooks": {"preToolUse": [{"command": "echo gated"}]}}, + ) + project = tmp_path / "project" + (project / ".cursor").mkdir(parents=True) + with pytest.raises(HookContractError, match=field): + HookIntegrator().integrate_hooks_for_target(KNOWN_TARGETS["cursor"], package, project) + assert not list(project.rglob("*.json")) + + +@pytest.mark.parametrize("owner", [None, [], {}]) +def test_malformed_inline_ownership_is_not_a_cleanup_grant(tmp_path: Path, owner: Any) -> None: + package = _package(tmp_path, {"preToolUse": [{"command": "echo package"}]}) + project = tmp_path / "project" + path = project / ".cursor/hooks.json" + _write_json( + path, + { + "version": 1, + "hooks": {"preToolUse": [{"command": "echo user", "_apm_source": owner}]}, + }, + ) + before = path.read_bytes() + with pytest.raises(HookContractError, match="invalid Cursor hook ownership"): + HookIntegrator().integrate_hooks_for_target(KNOWN_TARGETS["cursor"], package, project) + assert path.read_bytes() == before + + +@pytest.mark.parametrize("first", ["claude", "cursor"]) +@pytest.mark.parametrize("global_owner", [False, True], ids=["project-user", "global-owned"]) +def test_retirement_does_not_ignore_user_or_global_hooks( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + first: str, + global_owner: bool, +) -> None: + project, home = tmp_path / "project", tmp_path / "home" + monkeypatch.setenv("HOME", str(home)) + (project / ".cursor").mkdir(parents=True) + (home / ".cursor").mkdir(parents=True) + package = _package(tmp_path, {"PreToolUse": [_nested("echo original")]}) + hook = HookIntegrator() + hook.integrate_hooks_for_target(KNOWN_TARGETS[first], package, project) + if global_owner: + hook.integrate_hooks_for_target(KNOWN_TARGETS[first], package, home, user_scope=True) + else: + path = project / f".{first}" / ("settings.json" if first == "claude" else "hooks.json") + document = json.loads(path.read_text()) + event = "PreToolUse" if first == "claude" else "preToolUse" + entry = ( + _nested("echo user-overlap") if first == "claude" else {"command": "echo user-overlap"} + ) + document["hooks"][event].append(entry) + _write_json(path, document) + _write_json( + package.install_path / ".apm/hooks/gate.json", + {"hooks": {"PreToolUse": [_nested("echo user-overlap")]}}, + ) + second = "cursor" if first == "claude" else "claude" + package.package = APMPackage(name="cursor-gate", version="1.0.0", targets=[second]) + before = {p: p.read_bytes() for root in (project, home) for p in root.rglob("*") if p.is_file()} + with pytest.raises(HookContractError, match="Claude import"): + integrate_package_primitives( + package, + project, + targets=[KNOWN_TARGETS["claude"], KNOWN_TARGETS["cursor"]], + force=False, + managed_files=None, + integrators=IntegratorBundle( + prompt=None, + agent=None, + skill=SkillIntegrator(), + instruction=None, + command=None, + hook=hook, + ), + diagnostics=DiagnosticCollector(), + ) + assert { + p: p.read_bytes() for root in (project, home) for p in root.rglob("*") if p.is_file() + } == before + + +# --- Reused-plan preflight regression (#3129) ------------------------------ +# +# The up-front `preflight_hooks_for_targets` call and the per-target write +# boundary inside `_integrate_merged_hooks` used to share a single +# `DeployableSourcePlan.cursor_preflight_done` cache bit: once set True by +# one call, later per-target writes for the SAME plan object skipped the +# re-check entirely. A plan is not tied to one write: the same plan can be +# reused for a second `integrate_hooks_for_target` call in the same project +# after a Claude import file appears, or reused verbatim against a second +# project that already has one. Both cases must still be rejected. + + +def test_reused_plan_rejects_claude_import_added_after_upfront_preflight(tmp_path: Path) -> None: + """A Claude import appearing after the up-front preflight must still be caught.""" + package = _package(tmp_path, {"PreToolUse": [_nested("echo shared")]}) + project = tmp_path / "project" + (project / ".cursor").mkdir(parents=True) + plan = DeployableSourcePlan.create( + package, + [KNOWN_TARGETS["cursor"]], + skill_subset=None, + hooks_approved=True, + canvas_approved=False, + skip_bin=True, + ) + integrator = HookIntegrator() + + # Up-front preflight runs against a clean project: nothing to reject yet. + integrator.preflight_hooks_for_targets(package, project, plan) + + # A Claude import with the same shared action appears after that check, + # before the per-target write actually happens. + _write_json( + project / ".claude/settings.json", + {"hooks": {"PreToolUse": [_nested("echo shared")]}}, + ) + + with pytest.raises(HookContractError, match="Claude import"): + integrator.integrate_hooks_for_target( + KNOWN_TARGETS["cursor"], package, project, source_plan=plan + ) + + assert not (project / ".cursor/hooks.json").exists() + + +def test_reused_plan_rejects_overlap_in_a_different_project(tmp_path: Path) -> None: + """The same plan object reused for a second project must still be rejected.""" + package = _package(tmp_path, {"PreToolUse": [_nested("echo shared")]}) + project_a = tmp_path / "project-a" + project_b = tmp_path / "project-b" + (project_a / ".cursor").mkdir(parents=True) + (project_b / ".cursor").mkdir(parents=True) + _write_json( + project_b / ".claude/settings.json", + {"hooks": {"PreToolUse": [_nested("echo shared")]}}, + ) + plan = DeployableSourcePlan.create( + package, + [KNOWN_TARGETS["cursor"]], + skill_subset=None, + hooks_approved=True, + canvas_approved=False, + skip_bin=True, + ) + integrator = HookIntegrator() + + # First project is clean; the plan's up-front preflight and the write + # both succeed and must not poison later use of the same plan object. + integrator.preflight_hooks_for_targets(package, project_a, plan) + result = integrator.integrate_hooks_for_target( + KNOWN_TARGETS["cursor"], package, project_a, source_plan=plan + ) + assert result.files_integrated == 1 + + # Reusing the SAME plan for project_b, which already has an overlapping + # Claude import, must still be rejected -- not silently bypassed because + # the plan was already "preflighted" for project_a. + with pytest.raises(HookContractError, match="Claude import"): + integrator.integrate_hooks_for_target( + KNOWN_TARGETS["cursor"], package, project_b, source_plan=plan + ) + + assert not (project_b / ".cursor/hooks.json").exists() + + +def test_reused_plan_rejects_overlap_in_a_different_project_before_any_write( + tmp_path: Path, +) -> None: + """Switching projects BEFORE the plan's first native write must still reject. + + The sibling case above performs a real write for project_a before + switching to project_b, proving reuse-after-use is caught. This case + proves the narrower, earlier scenario: the plan only ran its up-front + ``preflight_hooks_for_targets`` for project_a -- no native write ever + happened there -- before being reused for project_b's own overlapping + import. Both project roots must stay exactly as they started. + """ + package = _package(tmp_path, {"PreToolUse": [_nested("echo shared")]}) + project_a = tmp_path / "project-a" + project_b = tmp_path / "project-b" + (project_a / ".cursor").mkdir(parents=True) + (project_b / ".cursor").mkdir(parents=True) + _write_json( + project_b / ".claude/settings.json", + {"hooks": {"PreToolUse": [_nested("echo shared")]}}, + ) + before_b = (project_b / ".claude/settings.json").read_bytes() + plan = DeployableSourcePlan.create( + package, + [KNOWN_TARGETS["cursor"]], + skill_subset=None, + hooks_approved=True, + canvas_approved=False, + skip_bin=True, + ) + integrator = HookIntegrator() + + # Up-front preflight only, against the clean project_a. No native write + # for project_a happens anywhere in this test. + integrator.preflight_hooks_for_targets(package, project_a, plan) + + # The same plan object -- "preflighted" only against project_a, never + # written anywhere yet -- is reused directly against project_b, which + # already has an overlapping Claude import. + with pytest.raises(HookContractError, match="Claude import"): + integrator.integrate_hooks_for_target( + KNOWN_TARGETS["cursor"], package, project_b, source_plan=plan + ) + + assert not (project_a / ".cursor/hooks.json").exists() + assert not (project_b / ".cursor/hooks.json").exists() + assert (project_b / ".claude/settings.json").read_bytes() == before_b + + +def test_reused_plan_still_succeeds_without_a_conflicting_import(tmp_path: Path) -> None: + """Control: the unconditional re-check must not over-reject the clean case.""" + package = _package(tmp_path, {"PreToolUse": [_nested("echo shared")]}) + project = tmp_path / "project" + (project / ".cursor").mkdir(parents=True) + plan = DeployableSourcePlan.create( + package, + [KNOWN_TARGETS["cursor"]], + skill_subset=None, + hooks_approved=True, + canvas_approved=False, + skip_bin=True, + ) + integrator = HookIntegrator() + + integrator.preflight_hooks_for_targets(package, project, plan) + result = integrator.integrate_hooks_for_target( + KNOWN_TARGETS["cursor"], package, project, source_plan=plan + ) + + assert result.files_integrated == 1 + assert (project / ".cursor/hooks.json").exists() + + +def test_reused_plan_preserves_unrelated_user_owned_claude_hooks(tmp_path: Path) -> None: + """The re-check must not flag or touch an unrelated, non-overlapping user hook.""" + package = _package(tmp_path, {"PreToolUse": [_nested("echo installed")]}) + project = tmp_path / "project" + (project / ".cursor").mkdir(parents=True) + claude = project / ".claude/settings.json" + _write_json(claude, {"hooks": {"PreToolUse": [_nested("echo unrelated-user-hook")]}}) + before = claude.read_bytes() + plan = DeployableSourcePlan.create( + package, + [KNOWN_TARGETS["cursor"]], + skill_subset=None, + hooks_approved=True, + canvas_approved=False, + skip_bin=True, + ) + integrator = HookIntegrator() + + integrator.preflight_hooks_for_targets(package, project, plan) + result = integrator.integrate_hooks_for_target( + KNOWN_TARGETS["cursor"], package, project, source_plan=plan + ) + + assert result.files_integrated == 1 + assert claude.read_bytes() == before + + +# --- Orchestration-tier (install-pipeline) coverage for the same guarantee - +# +# The tests above call ``HookIntegrator.preflight_hooks_for_targets`` and +# ``integrate_hooks_for_target`` directly. Real installs never do that: the +# production entry point is ``services.integrate_package_primitives``, which +# builds the ``DeployableSourcePlan`` itself, runs the up-front preflight, +# then dispatches each target's write -- all within ONE call. This section +# proves the guarantee holds through that real orchestration path, and that +# the per-write re-check inside ``_integrate_merged_hooks`` is load-bearing +# (not merely decorative) by disabling only it as a negative control. +# +# (The CLI/``apm install``-tier case -- going through the ``install`` Click +# command itself -- is covered separately in +# ``tests/integration/test_install_cli_cursor_claude_import_recheck_e2e.py``; +# this section stays scoped to the ``services.integrate_package_primitives`` +# dispatch tier.) + + +def _run_pipeline_with_late_claude_import( + tmp_path: Path, label: str +) -> tuple[HookContractError | None, Path]: + """Run one real ``integrate_package_primitives`` call, injecting a + conflicting Claude import between the real up-front preflight and the + real per-target write (both inside that single orchestrated call). + + No field or integrator is mocked; only the *timing* of the import write + is controlled, via a call-through wrapper around + ``HookIntegrator.preflight_hooks_for_targets`` so the import lands at the + exact real write boundary. Returns the raised ``HookContractError`` (or + ``None`` if the write was wrongly allowed) and the project root, so the + same assertion can be reused against both a real run and a scoped + mutant run. + """ + root = tmp_path / label + package = _package(root, {"PreToolUse": [_nested("echo shared")]}) + project = root / "project" + (project / ".cursor").mkdir(parents=True) + hook = HookIntegrator() + real_preflight = HookIntegrator.preflight_hooks_for_targets + + def _preflight_then_inject_claude_import( + self: HookIntegrator, *args: Any, **kwargs: Any + ) -> None: + real_preflight(self, *args, **kwargs) + _write_json( + project / ".claude/settings.json", + {"hooks": {"PreToolUse": [_nested("echo shared")]}}, + ) + + raised: HookContractError | None = None + with patch.object( + HookIntegrator, "preflight_hooks_for_targets", _preflight_then_inject_claude_import + ): + try: + integrate_package_primitives( + package, + project, + targets=[KNOWN_TARGETS["cursor"]], + force=False, + managed_files=None, + integrators=IntegratorBundle( + prompt=None, + agent=None, + skill=SkillIntegrator(), + instruction=None, + command=None, + hook=hook, + ), + diagnostics=DiagnosticCollector(), + ) + except HookContractError as exc: + raised = exc + return raised, project + + +def _assert_pipeline_rejected_the_late_import( + raised: HookContractError | None, project: Path +) -> None: + """The shared regression assertion, reused unmodified against a real run + and a scoped-mutant run so the mutant proof exercises the exact same + check the baseline must satisfy.""" + assert raised is not None, ( + "expected HookContractError for a Claude import appearing mid-install; " + "none was raised (missing refusal)" + ) + assert "Claude import" in str(raised) + assert not (project / ".cursor/hooks.json").exists(), ( + "native write must not occur when the rejection fires" + ) + + +def test_pipeline_rejects_claude_import_appearing_during_a_single_install_call( + tmp_path: Path, +) -> None: + """A Claude import written between the real up-front preflight and the + real per-target write -- both inside one ``integrate_package_primitives`` + call -- must still be rejected.""" + raised, project = _run_pipeline_with_late_claude_import(tmp_path, "baseline") + _assert_pipeline_rejected_the_late_import(raised, project) + + +def test_pipeline_control_disabling_only_the_per_write_recheck_loses_the_rejection( + tmp_path: Path, +) -> None: + """Mutation control: disabling ONLY the per-write re-check inside + ``_integrate_merged_hooks`` (not the up-front preflight, which must keep + running for real) must make the SAME regression assertion above FAIL, + for the expected missing-refusal reason -- proving that specific call is + what enforces the guarantee, not the up-front preflight alone. The + disabled state is scoped to one patched method for one call inside this + test only; it is never committed and the up-front preflight is never + touched. + """ + # Baseline sanity check first, with every real guard intact. + raised, project = _run_pipeline_with_late_claude_import(tmp_path, "before") + _assert_pipeline_rejected_the_late_import(raised, project) + + real_integrate_merged = HookIntegrator._integrate_merged_hooks + + def _integrate_merged_with_recheck_disabled(self: HookIntegrator, *args: Any, **kwargs: Any): + # Disable ONLY the per-write re-check, and only for the duration of + # this one real ``_integrate_merged_hooks`` call. The up-front + # ``preflight_hooks_for_targets`` call (which runs earlier, outside + # this patch's window) is never touched -- it ran for real above. + original = hook_integrator.preflight_cursor_hooks + hook_integrator.preflight_cursor_hooks = lambda *a, **k: None + try: + return real_integrate_merged(self, *args, **kwargs) + finally: + hook_integrator.preflight_cursor_hooks = original + + with patch.object( + HookIntegrator, "_integrate_merged_hooks", _integrate_merged_with_recheck_disabled + ): + mutant_raised, mutant_project = _run_pipeline_with_late_claude_import(tmp_path, "mutant") + with pytest.raises(AssertionError, match="missing refusal"): + _assert_pipeline_rejected_the_late_import(mutant_raised, mutant_project) + # Confirm the assertion failed for the expected reason (the native + # write went through), not some unrelated break. + assert mutant_raised is None + assert (mutant_project / ".cursor/hooks.json").exists() + + # Restored: the real guard is back in place for the next install. + restored_raised, restored_project = _run_pipeline_with_late_claude_import(tmp_path, "restored") + _assert_pipeline_rejected_the_late_import(restored_raised, restored_project) diff --git a/tests/unit/integration/test_dep_target_intersection.py b/tests/unit/integration/test_dep_target_intersection.py index d394f3fed..62afa3e7f 100644 --- a/tests/unit/integration/test_dep_target_intersection.py +++ b/tests/unit/integration/test_dep_target_intersection.py @@ -352,13 +352,19 @@ def _target_command_hook(command: str) -> dict: ``SessionStart`` and a trivial command survive event normalization across every merge harness, so the command string is a reliable per-file marker - regardless of a target's casing/matcher conventions. + regardless of a target's casing/matcher conventions. The matcher is the + wildcard ``"*"`` rather than an arbitrary word: ``SessionStart`` is not a + tool-use event, so Cursor's native contract only has a verified + translation for wildcard/empty matchers on non-tool lifecycle events + (see ``_cursor_matcher``); a non-wildcard placeholder would be rejected + by Cursor's strict preflight while being semantically meaningless for + every other harness too, so the wildcard is the correct value here, not + a weakening of this test's actual regression (per-target file routing + isolation, asserted below). """ return { "hooks": { - "SessionStart": [ - {"matcher": "startup", "hooks": [{"type": "command", "command": command}]} - ], + "SessionStart": [{"matcher": "*", "hooks": [{"type": "command", "command": command}]}], } } @@ -400,7 +406,20 @@ def _write_divergent_pair(package_path: Path, first: str, second: str) -> dict[s @pytest.mark.parametrize( ("first", "second"), - [(a, b) for i, a in enumerate(_MERGE_HARNESSES) for b in _MERGE_HARNESSES[i + 1 :]], + [ + (a, b) + for i, a in enumerate(_MERGE_HARNESSES) + for b in _MERGE_HARNESSES[i + 1 :] + # claude+cursor is intentionally excluded from this "any two harnesses + # diverge independently" matrix: Cursor's native/Claude-import + # coexistence preflight correctly refuses two per-target files that + # both declare the same event for this specific pair (same-event + # double-activation risk), unlike every other harness pair. That + # refusal is the PR's own feature and is covered directly by + # test_cursor_hook_native_contract.py's overlap tests, not by this + # generic routing-isolation matrix. + if {a, b} != {"claude", "cursor"} + ], ) def test_divergent_hook_files_route_per_target_under_multi_dep_targets( tmp_path: Path, first: str, second: str diff --git a/tests/unit/integration/test_hook_diagnostics.py b/tests/unit/integration/test_hook_diagnostics.py index a3a0b1820..2e1fc4696 100644 --- a/tests/unit/integration/test_hook_diagnostics.py +++ b/tests/unit/integration/test_hook_diagnostics.py @@ -68,8 +68,8 @@ def test_copilot_expects_camel(self): def test_claude_expects_pascal(self): assert _HOOK_EVENT_EXPECTED_CASING["claude"] == "PascalCase" - def test_cursor_expects_pascal(self): - assert _HOOK_EVENT_EXPECTED_CASING["cursor"] == "PascalCase" + def test_cursor_expects_camel(self): + assert _HOOK_EVENT_EXPECTED_CASING["cursor"] == "camelCase" def test_vscode_expects_pascal(self): assert _HOOK_EVENT_EXPECTED_CASING["vscode"] == "PascalCase" diff --git a/tests/unit/integration/test_hook_integrator.py b/tests/unit/integration/test_hook_integrator.py index ebd3fe688..c7e44179f 100644 --- a/tests/unit/integration/test_hook_integrator.py +++ b/tests/unit/integration/test_hook_integrator.py @@ -13,11 +13,12 @@ import shutil import tempfile from pathlib import Path -from unittest.mock import MagicMock +from unittest.mock import MagicMock, patch import pytest from apm_cli.core.deployment_state import MaterializationStatus +from apm_cli.hook_contract import HookContractError from apm_cli.integration.hook_command_paths import normalize_quoted_plugin_root from apm_cli.integration.hook_integrator import ( HookIntegrationResult, # noqa: F401 @@ -1282,18 +1283,18 @@ def test_integrate_hookify_cursor(self, temp_project): config = json.loads(hooks_path.read_text()) assert "hooks" in config assert config["version"] == 1 - assert "PreToolUse" in config["hooks"] - assert "PostToolUse" in config["hooks"] - assert "Stop" in config["hooks"] - assert "UserPromptSubmit" in config["hooks"] + assert "preToolUse" in config["hooks"] + assert "postToolUse" in config["hooks"] + assert "stop" in config["hooks"] + assert "beforeSubmitPrompt" in config["hooks"] # Ownership stays in the external APM sidecar, not the native schema. - assert "_apm_source" not in config["hooks"]["PreToolUse"][0] + assert "_apm_source" not in config["hooks"]["preToolUse"][0] sidecar = json.loads((temp_project / ".cursor" / "apm-hooks.json").read_text()) - assert sidecar["PreToolUse"][0]["_apm_source"] == "hookify" + assert sidecar["preToolUse"][0]["_apm_source"] == "hookify" # Verify rewritten paths point to .cursor/hooks/ (normalize separators) - cmd = config["hooks"]["PreToolUse"][0]["hooks"][0]["command"] + cmd = config["hooks"]["preToolUse"][0]["command"] assert ".cursor/hooks/hookify/hooks/pretooluse.py" in cmd.replace("\\", "/") def test_skips_when_no_cursor_dir(self, temp_project): @@ -1333,10 +1334,10 @@ def test_merge_into_existing_hooks_json(self, temp_project): assert len(config["hooks"]["afterFileEdit"]) == 1 assert config["hooks"]["afterFileEdit"][0]["command"] == "echo user-hook" # New hook added - assert "Stop" in config["hooks"] - assert "_apm_source" not in config["hooks"]["Stop"][0] + assert "stop" in config["hooks"] + assert "_apm_source" not in config["hooks"]["stop"][0] sidecar = json.loads((temp_project / ".cursor" / "apm-hooks.json").read_text()) - assert sidecar["Stop"][0]["_apm_source"] == "pkg" + assert sidecar["stop"][0]["_apm_source"] == "pkg" def test_additive_merge_same_event(self, temp_project): """Test that multiple packages can add hooks to the same event.""" @@ -1370,11 +1371,11 @@ def test_additive_merge_same_event(self, temp_project): integrator.integrate_package_hooks_cursor(pkg2_info, temp_project) config = json.loads((temp_project / ".cursor" / "hooks.json").read_text()) - # Both entries present under Stop - assert len(config["hooks"]["Stop"]) == 2 - assert all("_apm_source" not in entry for entry in config["hooks"]["Stop"]) + # Both entries present under native stop. + assert len(config["hooks"]["stop"]) == 2 + assert all("_apm_source" not in entry for entry in config["hooks"]["stop"]) sidecar = json.loads((temp_project / ".cursor" / "apm-hooks.json").read_text()) - assert [entry["_apm_source"] for entry in sidecar["Stop"]] == [ + assert [entry["_apm_source"] for entry in sidecar["stop"]] == [ "ralph-loop", "other-pkg", ] @@ -1469,18 +1470,105 @@ def test_cursor_version_emitted_on_fresh_install(self, temp_project): assert config.get("version") == 1 def test_cursor_existing_version_preserved(self, temp_project): - """A pre-existing "version" value in hooks.json must not be overwritten.""" + """Unsupported existing versions are rejected, not overwritten.""" hooks_path = temp_project / ".cursor" / "hooks.json" hooks_path.write_text(json.dumps({"version": 2, "hooks": {}})) pkg_info = self._setup_hookify_package(temp_project) integrator = HookIntegrator() - integrator.integrate_package_hooks_cursor(pkg_info, temp_project) + with pytest.raises(HookContractError, match="requires version 1"): + integrator.integrate_package_hooks_cursor(pkg_info, temp_project) config = json.loads(hooks_path.read_text()) assert config.get("version") == 2 + def test_cursor_existing_unknown_top_level_key_rejected(self, temp_project): + """Unsupported top-level keys Cursor would not load are rejected, not silently kept.""" + hooks_path = temp_project / ".cursor" / "hooks.json" + hooks_path.write_text(json.dumps({"version": 1, "hooks": {}, "unsupportedKey": True})) + + pkg_info = self._setup_hookify_package(temp_project) + integrator = HookIntegrator() + + with pytest.raises(HookContractError, match="unsupported top-level keys"): + integrator.integrate_package_hooks_cursor(pkg_info, temp_project) + + config = json.loads(hooks_path.read_text()) + assert config.get("unsupportedKey") is True + + def test_fallback_preflight_forwards_retiring_targets(self, temp_project): + """``_integrate_merged_hooks`` always re-checks Cursor/import sources + at its own write boundary, unconditionally, even when an up-front + ``preflight_hooks_for_targets`` call already ran for this plan + (#3129). That check must still honor ``retiring_targets`` or a + target slated for safe retirement this run can be misflagged as an + import-coexistence conflict. Verified at the exact plumbing + boundary (the forwarded kwarg), not by re-deriving the full + overlap-detection algorithm. + """ + from apm_cli.install.deployable_source_plan import DeployableSourcePlan + from apm_cli.integration.hook_integrator import _MERGE_HOOK_TARGETS + + pkg_info = self._setup_hookify_package(temp_project) + integrator = HookIntegrator() + # hook_source_selection=None means the up-front + # preflight_hooks_for_targets() gate is a no-op for this plan, so + # the unconditional check inside _integrate_merged_hooks is the + # ONLY preflight that runs for this single invocation. + source_plan = DeployableSourcePlan(source_root=temp_project, paths=frozenset()) + expected_retiring = frozenset({"claude"}) + + with patch("apm_cli.integration.hook_integrator.preflight_cursor_hooks") as mock_preflight: + integrator._integrate_merged_hooks( + _MERGE_HOOK_TARGETS["cursor"], + pkg_info, + temp_project, + source_plan=source_plan, + retiring_targets=expected_retiring, + ) + + mock_preflight.assert_called_once() + assert mock_preflight.call_args.kwargs["retiring_targets"] == expected_retiring + + def test_integrate_merged_hooks_rechecks_preflight_after_upfront_run(self, temp_project): + """Mutation-break proof for the #3129 cached-plan bypass. + + The up-front ``preflight_hooks_for_targets`` call and the per-target + write boundary inside ``_integrate_merged_hooks`` must each call + ``preflight_cursor_hooks`` independently. If a "run once per plan" + cache bit is reintroduced (the removed ``cursor_preflight_done`` + field), the per-target call below would be skipped and the mock's + total call count would stay at 1 instead of reaching 2. + """ + from apm_cli.install.deployable_source_plan import DeployableSourcePlan + from apm_cli.integration.hook_integrator import _MERGE_HOOK_TARGETS + from apm_cli.integration.targets import KNOWN_TARGETS + + pkg_info = self._setup_hookify_package(temp_project) + integrator = HookIntegrator() + source_plan = DeployableSourcePlan.create( + pkg_info, + [KNOWN_TARGETS["cursor"]], + skill_subset=None, + hooks_approved=True, + canvas_approved=False, + skip_bin=True, + ) + assert source_plan.hook_source_selection is not None + + with patch("apm_cli.integration.hook_integrator.preflight_cursor_hooks") as mock_preflight: + integrator.preflight_hooks_for_targets(pkg_info, temp_project, source_plan) + assert mock_preflight.call_count == 1 + + integrator._integrate_merged_hooks( + _MERGE_HOOK_TARGETS["cursor"], + pkg_info, + temp_project, + source_plan=source_plan, + ) + assert mock_preflight.call_count == 2 + # ─── Sync/cleanup tests ────────────────────────────────────────────────────── @@ -4135,8 +4223,8 @@ def test_claude_preserves_pascal_case_events(self, temp_project: Path) -> None: assert "PostToolUse" in hooks, "PascalCase key must be preserved" assert "postToolUse" not in hooks, "No duplicate camelCase key should appear" - def test_cursor_no_normalisation(self, temp_project_with_cursor: Path) -> None: - """Cursor target has no event-name mapping; PostToolUse passes through as-is.""" + def test_cursor_native_normalisation(self, temp_project_with_cursor: Path) -> None: + """Cursor maps the documented PostToolUse alias to its native event.""" from apm_cli.integration.targets import KNOWN_TARGETS project = temp_project_with_cursor @@ -4154,7 +4242,8 @@ def test_cursor_no_normalisation(self, temp_project_with_cursor: Path) -> None: HookIntegrator().integrate_hooks_for_target(target, pkg_info, project) hooks = self._read_cursor_hooks(project).get("hooks", {}) - assert "PostToolUse" in hooks, "PostToolUse must survive cursor integration unchanged" + assert "postToolUse" in hooks + assert "PostToolUse" not in hooks # ------------------------------------------------------------------ # Group D: Deduplication diff --git a/tests/unit/integration/test_hook_integrator_defect_regression.py b/tests/unit/integration/test_hook_integrator_defect_regression.py index baba3876c..59473d713 100644 --- a/tests/unit/integration/test_hook_integrator_defect_regression.py +++ b/tests/unit/integration/test_hook_integrator_defect_regression.py @@ -28,12 +28,12 @@ def _package_info(package_path: Path, name: str = "superpowers") -> PackageInfo: ) -def _session_start_hook(command: str = "echo hook") -> dict: +def _session_start_hook(command: str = "echo hook", *, matcher: str = "startup") -> dict: return { "hooks": { "SessionStart": [ { - "matcher": "startup", + "matcher": matcher, "hooks": [{"type": "command", "command": command}], } ] @@ -42,13 +42,13 @@ def _session_start_hook(command: str = "echo hook") -> dict: def _owned_hook_commands(path: Path) -> list[str]: - """Return commands from the nested hook fixture shape.""" + """Return commands from either native handler layout.""" document = json.loads(path.read_text(encoding="utf-8")) return [ handler["command"] for entries in document.get("hooks", {}).values() for entry in entries - for handler in entry.get("hooks", []) + for handler in entry.get("hooks", [entry]) ] @@ -563,7 +563,7 @@ def test_package_target_cleanup_uses_canonical_owner_not_shared_leaf( hooks_dir = install_path / ".apm" / "hooks" hooks_dir.mkdir(parents=True) (hooks_dir / "hooks.json").write_text( - json.dumps(_session_start_hook(command)), + json.dumps(_session_start_hook(command, matcher="")), encoding="utf-8", ) dependency = DependencyReference.parse(f"{owner}/hooks") @@ -620,7 +620,7 @@ def test_transitive_local_hook_markers_include_anchored_parent_identity( hooks_dir = install_path / ".apm" / "hooks" hooks_dir.mkdir(parents=True) (hooks_dir / "hooks.json").write_text( - json.dumps(_session_start_hook(command)), + json.dumps(_session_start_hook(command, matcher="")), encoding="utf-8", ) package = PackageInfo( @@ -751,7 +751,7 @@ def test_unambiguous_legacy_leaf_marker_migrates_to_canonical_owner( install_path = project / "apm_modules" / "org-a" / "hooks" hooks_dir = install_path / ".apm" / "hooks" hooks_dir.mkdir(parents=True) - hook = _session_start_hook("echo alpha") + hook = _session_start_hook("echo alpha", matcher="") (hooks_dir / "hooks.json").write_text(json.dumps(hook), encoding="utf-8") legacy_entry = _session_start_hook("echo legacy")["hooks"]["SessionStart"][0] (cursor / "hooks.json").write_text( diff --git a/tests/unit/integration/test_hook_naked_format.py b/tests/unit/integration/test_hook_naked_format.py index edcf4b11e..9f1101966 100644 --- a/tests/unit/integration/test_hook_naked_format.py +++ b/tests/unit/integration/test_hook_naked_format.py @@ -93,7 +93,7 @@ def test_claude_merges_naked_hook_format(tmp_path: Path) -> None: def test_cursor_merges_naked_hook_format(tmp_path: Path) -> None: - """Cursor hooks.json must contain the Stop entry, not ``{"hooks": {}}``.""" + """Cursor hooks.json must contain native stop, not an empty event map.""" pkg_info = _build_naked_hook_package(tmp_path / "pkg") project_root = tmp_path / "project" (project_root / ".cursor").mkdir(parents=True) @@ -107,8 +107,8 @@ def test_cursor_merges_naked_hook_format(tmp_path: Path) -> None: assert data.get("hooks", {}), ( f"Cursor hooks.json hooks must be non-empty for #1499 repro; got {data!r}" ) - assert "Stop" in data["hooks"], ( - f"Stop event must be merged into cursor hooks.json; got {data['hooks']!r}" + assert "stop" in data["hooks"], ( + f"Native stop must be merged into cursor hooks.json; got {data['hooks']!r}" ) assert result.files_integrated == 1 diff --git a/tests/unit/test_console_utils.py b/tests/unit/test_console_utils.py index 8aa646438..99f869e23 100644 --- a/tests/unit/test_console_utils.py +++ b/tests/unit/test_console_utils.py @@ -527,3 +527,67 @@ def test_set_stderr_false_restores_default(self): mod.set_console_stderr(False) assert mod._console_stderr is False assert mod._console_instance is None + + +class TestRichErrorNarrowWidthWrapping: + """Regression coverage for microsoft/apm#3129's Lifecycle Smoke flake. + + ``_rich_error`` renders through a real Rich ``Console`` which + word-wraps to the detected terminal width. Under a narrow / non-TTY + width (as in CI runners), a long diagnostic can have a literal + multi-word substring like "Claude import" split across a wrapped + line. Callers that assert on that literal substring must normalize + whitespace first; this test locks in that the wrap genuinely + happens at narrow widths and that whitespace-normalization recovers + the full semantic phrase regardless of where the wrap lands. + """ + + def setup_method(self): + from apm_cli.utils.console import _reset_console + + _reset_console() + + def teardown_method(self): + from apm_cli.utils.console import _reset_console + + _reset_console() + + def test_long_diagnostic_wraps_but_normalizes_back(self): + import io + + from rich.console import Console + + from apm_cli.utils.console import _rich_error + + # Mirrors the actual HookContractError message raised by + # hook_native_formats.py for a Cursor/Claude import-coexistence + # conflict (#3129); this is the real text whose wrap broke the + # test's substring assertion. + message = ( + "Cannot install hooks for cursor-native: Cursor native hooks " + "overlap Claude import and may run twice. Select one hook " + "target per dependency; existing Claude hooks can use " + "Cursor's third-party import instead. No import setting was " + "changed. Use supported Cursor-native hooks or a single " + "Claude-import route, then reinstall." + ) + buffer = io.StringIO() + narrow_console = Console(file=buffer, width=20, force_terminal=False) + with patch( + "apm_cli.utils.console._get_console", + return_value=narrow_console, + ): + _rich_error(message, symbol="error") + + rendered = buffer.getvalue() + # The narrow width must actually force a wrap for this + # regression test to mean anything -- otherwise it would pass + # even if the wrap-robustness fix were reverted. + assert "\n" in rendered.strip() + # The raw literal substring may be split across the wrap point; + # whitespace-normalizing (collapsing all runs of whitespace, + # including embedded newlines, to single spaces) must always + # recover the full semantic phrase. + normalized = " ".join(rendered.split()) + assert "Claude import" in normalized + assert "No import setting was changed" in normalized