diff --git a/.apm/architecture/owners/install-deployment.json b/.apm/architecture/owners/install-deployment.json index 7bbb70e14..25c35c706 100644 --- a/.apm/architecture/owners/install-deployment.json +++ b/.apm/architecture/owners/install-deployment.json @@ -85,6 +85,27 @@ "selectors": ["src/apm_cli/integration/base_integrator.py"], "guards": ["install-deployment-base-integrator"] }, + { + "id": "executable-trust-context", + "decision": "Executable trust context and per-package approval", + "owner": "security/executables.py (exec_trust_context_for_project, resolve_exec_decision)", + "selectors": ["src/apm_cli/security/executables.py"], + "guards": ["install-deployment-executable-trust-context"] + }, + { + "id": "lsp-target-contract", + "decision": "LSP target path, shape, and reserved Claude namespace", + "owner": "integration/lsp_integrator.py (LSPIntegrator)", + "selectors": ["src/apm_cli/integration/lsp_integrator.py"], + "guards": ["install-deployment-lsp-target-contract"] + }, + { + "id": "lsp-lifecycle", + "decision": "LSP lifecycle orchestration and reconciliation", + "owner": "install/lsp/integration.py", + "selectors": ["src/apm_cli/install/lsp/integration.py"], + "guards": ["install-deployment-lsp-lifecycle"] + }, { "id": "post-uninstall-dependency-reachability", "decision": "Post-uninstall dependency reachability", diff --git a/CHANGELOG.md b/CHANGELOG.md index 19c858aba..bd5004db7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `apm uninstall` now removes MCP servers only from recorded owning runtimes, accepts JetBrains Copilot JSONC, and reports target cleanup failures after attempting every owner. (by @aryansk, fixes #2551) (#2591) +- Claude project LSP servers now load from a discoverable APM-managed plugin + manifest instead of a path Claude Code ignored. Upgrade users can rerun + `apm install --target claude`; legacy + project-root `.lsp.json` files remain unchanged for manual review, and target + changes, package uninstall, and later executable denial remove only LSP + entries that APM created. (#2733) - `apm install` now preserves previously deployed skills when package integration is skipped instead of treating them as stale cleanup candidates. (#2758) @@ -47,6 +53,56 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 lockfile drift in projects that also carry MCP state. APM trusts the locked type before remote materialization and validates the skill shape when present, for repository-root and subdirectory skills. (#2446) +- OpenCode MCP generation now preserves safe passthrough fields while preventing + custom fields from injecting the modeled `environment` alias. (by @aryansk, + fixes #2510) (#2593) +- Hook commands such as `"${CLAUDE_PLUGIN_ROOT}"/hooks/probe.py` now rewrite to + `"${CLAUDE_PLUGIN_ROOT}/hooks/probe.py"` and warn when a supported plugin-root + placeholder remains unresolved instead of silently deploying a dead hook. + OpenAPM v0.1 (`docs/src/content/docs/specs/openapm-v0.1.md#req-tg-012`) binds + the behavior. + (by @MohammedAlkindi; closes #2639) (#2645) +- `apm uninstall --global` now cleans removed-only target files before deleting their ownership state, while preserving files owned by surviving packages. (#2658) +- Generic marketplace Git now prevents platform tokens from reaching native + credential-helper subprocesses while preserving HTTPS helper access; HTTP and + HTTPS-to-HTTP rewrites suppress credentials, and SSH is token-free and + noninteractive. (by @aryansk, #2594) +- Windows binary is now Authenticode-signed in the release workflow, eliminating + the `Trojan:Script/Wacatac.H!ml` Windows Defender false positive on unsigned + PyInstaller bundles. (#2435) +- Multi-target `apm compile` now avoids repeating expensive project analysis + for each target, making multi-target runs scale like single-target runs + without changing generated output. (closes #2482) +- `deployed-files-present` no longer false-positives on gitignored deploy + paths (e.g. `.agents/`), enabling `apm audit --ci` to pass on a fresh + checkout when deployed outputs are intentionally not committed. (closes + #2452, thanks @sergio-sisternes-epam) +- YAML expansion guard no longer rejects large anchor-free lockfiles (150K+ + entries) with a false-positive "billion-laughs" error. APM-generated + lockfiles with no anchors or aliases now load without error. (#2389) +- `apm install` no longer skips the credential retry on non-English machines. + Git localises its diagnostics through gettext, so a translated stderr made an + authentication failure unrecognisable and private-repo installs failed with + misleading network guidance. Git subprocesses in the authentication retry + path now run with `LC_ALL=C` and `LANGUAGE=C`. (by @Naofel-eal, closes #2533) +- Claude project LSP servers now load from a discoverable APM-managed plugin + manifest instead of a path Claude Code ignored. Upgrade users can rerun + `apm install --target claude`; legacy + project-root `.lsp.json` files remain unchanged for manual review, and target + changes, package uninstall, and executable denial revoke only recorded + APM-owned entries. (#2733) +- `apm pack` now reports unavailable remote package metadata, exposes + certifiability in JSON, prevents `--check-clean` from certifying degraded + regeneration, and lets `--strict-metadata` fail before writes. (closes #2524) +- `apm pack --check-clean` is now read-only and detects marketplace drift + without overwriting artifacts. Release pipelines that also produce artifacts + must run `apm pack` separately; see + [Releasing from any CI](docs/src/content/docs/producer/releasing-from-any-ci.md#the-canonical-sequence). + (by @danielmeppiel, closes #2727, #2730) +- `apm update` now retains full-SHA pins without an eligible stable annotated + semver tag while continuing unrelated updates; malformed remote tag records + still fail before writes. The contract is recorded in `openapm-v0.1.md`. + (#2667) - `apm install -g --mcp NAME` now creates or updates the user manifest and deploys only to global-capable runtimes instead of rejecting `--global`. Registry identities are validated before user-state writes, fail closed when @@ -68,6 +124,43 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 must run `apm pack` separately; see [Releasing from any CI](docs/src/content/docs/producer/releasing-from-any-ci.md#the-canonical-sequence). (by @danielmeppiel, closes #2727, #2730) +- Claude project LSP installs now write a discoverable APM-managed plugin + manifest. Upgrade users can rerun `apm install --target claude`; legacy + project-root `.lsp.json` files remain unchanged for manual review, and target + changes, package uninstall, and executable denial revoke only recorded + APM-owned entries. (#2733) +- Distributed `apm compile` now reconciles existing managed-section + `AGENTS.md` files without overwriting hand-authored content, generates new + placements safely, and never discovers, writes, or cleans content across + nested Git repository or linked-worktree boundaries. (by @aryansk; fixes + #2560 and #2713) (#2578) +- Windows admin lifecycle policies now resolve from `%ProgramData%` instead of + assuming `C:\ProgramData`, while retaining the historical fallback. + (by @lukiod; closes #2684) (#2686) +- Claude project LSP installs now write a discoverable APM-managed plugin + manifest. Upgrade users can rerun `apm install --target claude`; legacy + project-root `.lsp.json` files remain unchanged for manual review. (#2733) +- Marketplace installs now materialize catalog-only LSP and MCP metadata + without requiring a package manifest in the downloaded source + (by @lkshrk, #2709). +- Private `github.com` subdirectory packages now populate the persistent Git +- Private `github.com` packages now populate the persistent Git + cache through repository-scoped credential fallback without storing + credentials in cache keys or remote URLs. (#2722) +- Plugin refreshes now keep the existing package and its registered hooks live + while replacement content downloads and validates. Failed refreshes retain + the prior package instead of accepting stale content. (#2723) +- Successful installs now remove inactive resolution staging directories left + by interrupted earlier runs while preserving active and unrelated entries. + (closes #2716) +- Successful installs now safely clean up temporary backups left by interrupted + lock-aware runs without disturbing active installs or unrelated files. Legacy + lockless backups are preserved with manual recovery guidance. (#2720) +- `apm doctor` now reports malformed project `executables` configuration as an + actionable informational warning instead of omitting the check. (#2719) +- `apm doctor` now reports malformed project `executables` or deprecated + `allowExecutables` configuration as an actionable informational warning + instead of omitting the check. (#2719) - `apm doctor` now reports malformed project executable-trust configuration under either `executables` or the deprecated `allowExecutables` key as an actionable informational warning instead of omitting the check. (#2719) diff --git a/docs/src/content/docs/consumer/deploy-a-bundle.md b/docs/src/content/docs/consumer/deploy-a-bundle.md index 63fb436fd..ed556f608 100644 --- a/docs/src/content/docs/consumer/deploy-a-bundle.md +++ b/docs/src/content/docs/consumer/deploy-a-bundle.md @@ -54,8 +54,8 @@ with a warning unless policy requires hashes. With ## How the install works :::note[Governed offline installs] -When the policy cache is warm, offline bundles still apply target, MCP, and -integrity rules without network access. +When the policy cache is warm, offline bundles still apply target, MCP, LSP, +canvas, and integrity rules without network access. ::: ``` @@ -70,20 +70,24 @@ Steps APM runs: 1. **Detect.** Path exists and contains `plugin.json` at the bundle root (zip archives and legacy tarballs are extracted to a temp directory first). 2. **Apply cached policy.** Resolve the actual targets, then apply any cached - org policy to bundle MCP entries and targets before files are written. - Bundle install never fetches policy from the network. `--no-policy` skips - this local gate for one invocation. + org policy to bundle MCP, LSP, canvas, and target rules before files are + written. Bundle install never fetches policy from the network. `--no-policy` + skips this local gate for one invocation. 3. **Verify integrity.** Hash every file listed in `pack.bundle_files`; reject any symlink, hash mismatch, or unlisted file. -4. **Deploy.** Map `agents/`, `skills/`, `commands/`, `hooks/` into the - harness layout for each `--target` you passed. For plugin skill declaration - precedence, see [Package Types](../../reference/package-types/#plugin-collection-pluginjson). +4. **Deploy.** Map `agents/`, `skills/`, `commands/`, `hooks/`, LSP metadata, + and canvas extensions into the harness layout for each `--target` you + passed. For plugin skill declaration precedence, see + [Package Types](../../reference/package-types/#plugin-collection-pluginjson). 5. **Record.** Write a lockfile entry under the project's `apm.lock.yaml` so [drift detection](../drift-and-secure-by-default/) can audit the deployed files later. `apm.yml` is never touched. Re-running the same command re-deploys (use `--force` to overwrite locally-edited files). +If executable trust blocks bundle MCP, LSP, or canvas content, copy the exact +`name#version@sha256:` key printed by `apm install` into +`executables.allow`; changed bundle bytes require a new key. For centrally governed installs, warm the policy cache before distributing an offline bundle. See [APM policy](../../enterprise/apm-policy/). diff --git a/docs/src/content/docs/consumer/install-lsp-servers.md b/docs/src/content/docs/consumer/install-lsp-servers.md index a1efaffc3..97c33268b 100644 --- a/docs/src/content/docs/consumer/install-lsp-servers.md +++ b/docs/src/content/docs/consumer/install-lsp-servers.md @@ -23,6 +23,10 @@ for runtime-specific config details. Declare an LSP server in `apm.yml` and run `apm install`: +For this root-manifest example, install `gopls` separately and make sure it is +available on `PATH`; APM configures the runtime but does not install external +language-server executables. + ```yaml dependencies: lsp: @@ -34,13 +38,35 @@ dependencies: ``` ```bash -apm install +apm install --target claude ``` -APM writes runtime-specific config for each detected target. Claude Code -uses `.lsp.json` or `~/.claude.json`; Copilot CLI uses `.github/lsp.json` -or `~/.copilot/lsp-config.json`. The runtime starts the configured -language servers automatically. +APM writes runtime-specific config for each detected target. At project scope, +Claude Code discovers LSP servers from the APM-managed plugin manifest at +`.claude/skills/apm-lsp/.claude-plugin/plugin.json`; user-scope installs use +`~/.claude/skills/apm-lsp/.claude-plugin/plugin.json`. Copilot CLI uses +`.github/lsp.json` or `~/.copilot/lsp-config.json`. This generated shape +matches each runtime's documented discovery contract. + +Claude skills-directory plugin discovery requires Claude Code v2.1.157 or +newer. For project installs, accept Claude Code's workspace-trust prompt; LSP +servers start only after you trust the workspace. Start Claude from the +repository root so its primary working directory contains `.claude/skills/`: +project-scope skills-directory plugins do not walk up from a subdirectory to +the repo root. Personal-scope plugins under your home directory have no +workspace-trust gate. After APM reports that it configured or removed Claude +LSP servers, restart Claude Code or run `/reload-plugins` (use +`/reload-plugins --force` when Claude requests it). Open a file matching a +configured extension and confirm its LSP-backed diagnostics or navigation work +before relying on the integration. If another enabled Claude LSP server already +claims the same file extension, Claude uses the first registered server for +that extension and the others never start; for example, an APM-declared `.py` +server can lose to an installed `pyright-lsp`. + +If an earlier APM version created a project-root `.lsp.json`, APM leaves it +unchanged because it may contain user-owned entries. Claude Code does not use +that file for project plugin discovery. Review it, migrate any entries you +still need, then remove it. ## The `lsp:` section in apm.yml @@ -74,18 +100,21 @@ The full field reference is in the | Runtime | Project file | User file (`-g`) | Language map key | |---|---|---|---| -| Claude Code | `.lsp.json` | `~/.claude.json` `lspServers` | `extensionToLanguage` | +| Claude Code | `.claude/skills/apm-lsp/.claude-plugin/plugin.json` `lspServers` | `~/.claude/skills/apm-lsp/.claude-plugin/plugin.json` `lspServers` | `extensionToLanguage` | | GitHub Copilot CLI | `.github/lsp.json` `lspServers` | `~/.copilot/lsp-config.json` `lspServers` | `fileExtensions` | -**Claude Code project-scope `.lsp.json` example:** +**Claude Code project-scope plugin manifest example:** ```json { - "gopls": { - "command": "gopls", - "args": ["serve"], - "extensionToLanguage": { - ".go": "go" + "name": "apm-lsp", + "lspServers": { + "gopls": { + "command": "gopls", + "args": ["serve"], + "extensionToLanguage": { + ".go": "go" + } } } } @@ -108,7 +137,8 @@ The full field reference is in the ``` User-scope files keep the same runtime-specific server shape under their -`lspServers` section. +`lspServers` section. Claude skills-directory plugins are auto-discovered, so +APM does not write an `enabledPlugins` entry. ## Required and optional fields @@ -116,7 +146,7 @@ Two fields are required for every LSP server definition (object form): | Field | Type | Description | |---|---|---| -| `command` | `string` | Binary to execute. Must be on `$PATH` or a relative path. | +| `command` | `string` | Binary to execute. Must resolve from `$PATH` or use an absolute or relative path. | | `extensionToLanguage` | `map` | Maps file extensions to LSP language identifiers (e.g. `".go": "go"`). | Optional fields give you finer control: @@ -142,33 +172,42 @@ entries, APM collects them transitively after installation. Direct transitive package both declare a server with the same name, the root definition wins. -Unlike MCP, LSP has no registry vs self-defined distinction. All -LSP servers from installed packages are treated as trusted. +Unlike MCP, LSP has no registry vs self-defined distinction. LSP commands from +dependency packages pass the executable trust gate for their declaring package +when the gate is enabled. Approve the package with `apm approve ` +before APM exposes its server to a supported runtime. Without a project or org +`executables` opt-in, the compatibility default permits dependency +executables. Root-project LSP declarations are trusted as local project +content. ## Stale server cleanup When a previously installed LSP server is no longer declared by any dependency, APM removes it from the target runtime configs it manages. The lockfile tracks which servers APM manages, so hand-added servers are -never touched. +never touched. When cleanup removes the last managed server from an otherwise +empty APM-owned Claude project plugin, APM deletes the plugin and its empty +`apm-lsp` directory. ## Lockfile -`apm install` persists two fields in `apm.lock.yaml`: - -- `lsp_servers` -- sorted list of APM-managed server names. -- `lsp_configs` -- server-name-to-config baseline for drift detection. - -See the [Lockfile specification](../../reference/lockfile-spec/). +`apm install` records resolved LSP configuration, declaration ownership, and +target ownership in `apm.lock.yaml`. These fields let lifecycle commands +reconcile only APM-owned entries. See the +[Lockfile specification](../../reference/lockfile-spec/) for the canonical +field definitions. ## Plugin extraction -When APM installs a plugin that contains `lspServers` in `plugin.json` -or a `.lsp.json` file, the LSP servers are automatically extracted and -wired into the install pipeline. Plugin `.lsp.json` files may use either -a flat server map or a `{ "lspServers": { ... } }` envelope. The +When APM installs a plugin, it extracts LSP servers from an inline or +file-valued `lspServers` entry in `plugin.json`, or auto-discovers +`com.microsoft.apm/lsp.json`, `lsp.json`, or `.lsp.json`. The servers are +then wired into the install pipeline. Plugin LSP files may use either a flat +server map or a `{ "lspServers": { ... } }` envelope. The `${CLAUDE_PLUGIN_ROOT}` placeholder in server configs is replaced with the absolute plugin path for legacy Claude Code plugin compatibility. +These are source files shipped by a dependency package, distinct from the +`.claude-plugin/plugin.json` that APM generates for Claude discovery. Plugins authored for Copilot CLI may use `fileExtensions` instead of `extensionToLanguage` and `warmupTimeoutMs` instead of `startupTimeout`; APM normalizes those aliases before validation. A non-null canonical value @@ -190,7 +229,7 @@ success. | Runtime | LSP support | |---|---| -| Claude Code | `.lsp.json` / `~/.claude.json` | +| Claude Code | `.claude/skills/apm-lsp/.claude-plugin/plugin.json` / `~/.claude/skills/apm-lsp/.claude-plugin/plugin.json` | | GitHub Copilot CLI | `.github/lsp.json` / `~/.copilot/lsp-config.json` | | Others | Not yet supported | diff --git a/docs/src/content/docs/enterprise/security.md b/docs/src/content/docs/enterprise/security.md index 8cfa76120..72df4f4c6 100644 --- a/docs/src/content/docs/enterprise/security.md +++ b/docs/src/content/docs/enterprise/security.md @@ -419,10 +419,13 @@ across targets. ## Executable trust gate -APM blocks executable primitives from dependency packages by default: hooks, -`bin/` executables, self-defined MCP servers (`registry: false`), and canvas -extensions. Text primitives (skills, agents, instructions) are never gated, and -local root `.apm/` content is always trusted. +When the executable trust gate is enabled, APM blocks unapproved executable +primitives from dependency packages: hooks, `bin/` executables, self-defined +MCP servers (`registry: false`), LSP servers, and canvas extensions. The gate is +opt-in for backward compatibility; a project `executables:` block or org policy +enables it. Text primitives (skills, agents, instructions) are never gated, and +local root `.apm/` content is always trusted. Supported runtimes may start an +approved generated LSP command automatically after install. Trust is expressed through one noun, `executables`, across three layers, and the install gate and `apm audit` resolve it through a single deny-wins, @@ -445,14 +448,17 @@ first-match-wins ladder: - **Project** (`apm.yml` `executables.{allow,deny}`) is committed admin trust, shared with the team. - **User** (`~/.apm/config.json` `executables.{allow,deny}`) is the lowest - authority -- a machine-local override that can only narrow, never widen past - an org or project deny. + authority. `apm approve --user` can grant machine-local trust when no org or + project deny blocks it; `apm deny --user` narrows trust on one machine. Personal consent can never widen past an org deny, and the default (rung 7) is **gated pending approval** -- a package with executables and no opinion anywhere is parked until approved, not hard-denied. This release ships no `enforce` -mandate runtime, no signing, and no content-hash binding; an org -`executables.enforce` rung degrades to `recommend`. +mandate runtime or package signing; an org `executables.enforce` rung degrades +to `recommend`. Local bundle MCP, LSP, and canvas approvals are narrower: APM +prints an exact `executables.allow` key containing the bundle's SHA-256 content +digest, so another bundle that claims the same package name cannot inherit that +consent. Each locked dependency records its resolved state in the `exec_status` field of `apm.lock.yaml` (`deployed`, `gated_pending_approval`, `denied`, or `absent`). diff --git a/docs/src/content/docs/reference/cli/approve.md b/docs/src/content/docs/reference/cli/approve.md index a64d8d19b..7cb44121e 100644 --- a/docs/src/content/docs/reference/cli/approve.md +++ b/docs/src/content/docs/reference/cli/approve.md @@ -14,21 +14,22 @@ apm policy explain ## Description -APM blocks executable primitives (hooks, `bin/` executables, self-defined MCP -servers, and canvas extensions) from dependency packages by default. Trust is -expressed through one noun, `executables`, across three layers: +When the executable gate is enabled, APM blocks unapproved executable +primitives (hooks, `bin/` executables, self-defined MCP servers, LSP servers, +and canvas extensions) from dependency packages. Trust is expressed through +one noun, `executables`, across three layers: | Layer | Store | Who manages it | Committed? | Authority | |-------|-------|----------------|------------|-----------| | Project | `apm.yml` `executables.{allow,deny}` | Maintainer / CI setup (`apm approve`/`apm deny`) | Yes | Admin (shared) | -| User | `~/.apm/config.json` `executables.{allow,deny}` | `apm approve --user` / `apm deny --user` | No | Lowest; can only narrow | +| User | `~/.apm/config.json` `executables.{allow,deny}` | `apm approve --user` / `apm deny --user` | No | Lowest; below org/project deny | | Org | `apm-policy.yml` `executables:` | Org admin | Yes (policy repo) | Ceiling on deny | `apm approve` adds a grant; [`apm deny`](../deny/) adds a block. By default, both commands write the **project** `apm.yml` (committed, so the whole team inherits the decision). `--user` writes your personal -`~/.apm/config.json` instead -- a machine-local override that can only narrow -trust, never widen past an org or project deny. +`~/.apm/config.json` instead -- a machine-local grant with lower authority than +org or project denies. Use `apm deny --user` to narrow trust on one machine. Text primitives (skills, agents, instructions) are never gated. Local project content (the root `.apm/` directory) is always trusted. @@ -40,6 +41,7 @@ content (the root `.apm/` directory) is always trusted. | Hooks (`.apm/hooks/`, `hooks/`) | Yes | Auto-fire in IDE on lifecycle events | | Bin executables (`bin/`) | Yes | Deployed to agent PATH via symlinks | | MCP servers (self-defined) | Yes | `registry: false` servers write to IDE MCP config | +| LSP servers | Yes | Supported runtimes may start generated server commands automatically | | Canvas extensions (`.apm/extensions/`) | Yes | Deploys executable Node.js to IDE extensions | | Text primitives (skills, agents, instructions) | No | No code execution risk | @@ -64,9 +66,10 @@ pending approval**, not a hard deny: a package with executables and no opinion anywhere is parked until you approve it, and `apm install` still succeeds (see [`apm install`](../install/)). -There is no `enforce` mandate runtime, no cryptographic signing, and no -content-hash binding in this release. An org `executables.enforce` rung -degrades to `recommend` (allowed but still overridable by a deny). +There is no `enforce` mandate runtime or cryptographic signing in this release. +An org `executables.enforce` rung degrades to `recommend` (allowed but still +overridable by a deny). Local bundle MCP, LSP, and canvas approvals are bound to +the exact SHA-256 content key printed by `apm install`. ### The gate opt-in @@ -117,12 +120,14 @@ executables: "owner/repo#1.2.0": hooks: true bin: true + lsp: true deny: "evil/pkg": hooks: true mcp: true bin: true canvas: true + lsp: true ``` The legacy top-level `allowExecutables:` block is **deprecated**. It is still @@ -135,10 +140,12 @@ The personal store uses the same shape under `executables` in **removed**; its contents are migrated into `~/.apm/config.json` automatically on first read. -Grant keys are package-scoped in v1: a bare `owner/repo` key and a -`owner/repo#1.2.0` key both match the package name regardless of the installed -version. Use the versioned form for audit readability, not as a per-release -trust boundary. +Ordinary dependency grant keys are package-scoped in v1: a bare `owner/repo` +key and an `owner/repo#1.2.0` key both match the package name regardless of the +installed version. Use that versioned form for audit readability, not as a +per-release trust boundary. Local bundle grants are different: copy the full +`name#version@sha256:` key from the install diagnostic. A changed local +bundle does not inherit the prior grant. ## Examples diff --git a/docs/src/content/docs/reference/cli/install.md b/docs/src/content/docs/reference/cli/install.md index deba5ab09..64cef7b5b 100644 --- a/docs/src/content/docs/reference/cli/install.md +++ b/docs/src/content/docs/reference/cli/install.md @@ -119,6 +119,11 @@ in `apm.yml`, then run `apm install` again. - **Auto-bootstrap.** `apm install ` with no `apm.yml` creates a minimal one. Its name comes from the current directory (or home directory for global installs) and falls back to `my-project` if that derived name is invalid. Bare `apm install` with no `apm.yml` exits with a hint to run `apm init` or `apm install `. - **Target persistence on bootstrap.** When `--target` maps to recognized manifest targets, those target(s) are persisted to the new manifest's `targets:` field so a later bare `apm update` redeploys to the same targets without re-specifying `--target`. - **One effective target.** Package primitives, MCP servers, and LSP servers consume one target decision per invocation: `--target` > `apm.yml targets:` > `apm config set target ...` > auto-detect. A saved target therefore applies to `apm install`, `apm install --mcp`, and later `apm update` runs without another flag. +- **Claude LSP discovery.** Project installs write the APM-managed plugin at + `.claude/skills/apm-lsp/.claude-plugin/plugin.json`; global installs write + `~/.claude.json`. Existing project-root `.lsp.json` files are preserved for + manual review because they may contain user-owned entries. See + [Install LSP servers](../../../consumer/install-lsp-servers/). - **Required service writes fail loudly.** If MCP or LSP work is declared but no target can be resolved, install exits non-zero before changing the manifest, package deployment, or native service config. A native MCP/LSP config write failure also exits non-zero with the failed target and a permissions/path next step. A successful direct `--mcp` add never reports `Install interrupted`. - **Direct registry lookup fails closed.** Registry-form MCP entries (`apm install --mcp NAME` with no `--url` and no post-`--` command) resolve one unique registry identity before writing the manifest or user config. An unreachable registry, missing identity, or ambiguous bare server name exits non-zero without changing state. - **Diff-aware.** Packages whose ref or version changed in `apm.yml` are re-downloaded automatically. MCP servers with matching config are skipped (`already configured`); changed config is re-applied (`updated`). diff --git a/docs/src/content/docs/reference/cli/uninstall.md b/docs/src/content/docs/reference/cli/uninstall.md index ebe4d0b79..0dcd17172 100644 --- a/docs/src/content/docs/reference/cli/uninstall.md +++ b/docs/src/content/docs/reference/cli/uninstall.md @@ -96,12 +96,15 @@ What gets removed, in order: 4. Transitive dependencies that no remaining package depends on (npm-style pruning, computed from `apm.lock.yaml`). A transitive dependency still declared by any surviving package is preserved, even when two packages share it (a diamond-shaped install). If a surviving package's manifest can't be read, APM keeps every remaining candidate for that run rather than guessing -- re-run with `--verbose` to see which manifest failed, then fix or restore it and re-run to complete cleanup. 5. Every remaining file in the lockfile's `deployed_files` for the removed packages and pruned orphans, across configured target-owned folders such as `.github/`, `.claude/`, `.grok/`, and `.agents/`. 6. Hook entries inside `.claude/settings.json`, `.cursor/hooks.json`, `.gemini/settings.json`, and `.kiro/hooks/` that the removed packages contributed. Remaining packages -- including transitive dependencies still required by another package -- have their hook entries rebuilt from the post-removal lockfile. -7. MCP servers contributed only by the removed packages. For current lockfiles, - cleanup touches only the runtimes recorded as owners in `mcp_target_servers`. - An explicitly empty ownership map is a no-op. Older lockfiles adopt only - self-defined entries that exactly match their stored configuration baseline. - Cleanup attempts every owning runtime before exiting nonzero on failure. Fix - the reported configs, then run `apm install` to reconcile stale entries. +7. MCP and LSP servers contributed only by the removed packages. For current + lockfiles, MCP cleanup touches only the runtimes recorded as owners in + `mcp_target_servers`. An explicitly empty ownership map is a no-op. Older + lockfiles adopt only self-defined entries that exactly match their stored + configuration baseline. When another surviving package declares the same LSP + server name, ownership transfers to that package instead of deleting the + shared entry. Cleanup attempts every owning runtime before exiting nonzero on + failure. Fix the reported configs, then run `apm install` to reconcile stale + entries. 8. The lockfile entries themselves. If no dependencies remain, `apm.lock.yaml` is deleted. 9. Empty parent directories left behind by the cleanup. @@ -113,6 +116,11 @@ If a target-scoped file owned only by a removed package was edited or cannot be deleted, uninstall lists the retained paths and exits before changing `apm.yml`, `apm.lock.yaml`, or package modules. Resolve the listed files and retry. +If safe LSP cleanup fails after package removal, uninstall exits nonzero, +preserves the conflicting configuration, and tells you to repair the path or +ownership conflict. Run `apm install` to reconcile project state, or +`apm install --global` after a global uninstall. + `_local/` is resolved from the manifest and lockfile metadata that produced the `apm deps list` row. APM does not reinterpret it as `owner/repo` or rebuild an absolute path. If two declared local dependencies have the same portable key, diff --git a/docs/src/content/docs/reference/lockfile-spec.md b/docs/src/content/docs/reference/lockfile-spec.md index b7317e249..52f1ba7d8 100644 --- a/docs/src/content/docs/reference/lockfile-spec.md +++ b/docs/src/content/docs/reference/lockfile-spec.md @@ -115,6 +115,11 @@ lsp_configs: args: ["--stdio"] extensionToLanguage: ".py": python +lsp_target_servers: + claude: + - pyright +lsp_config_provenance: + pyright: "project:." local_deployed_files: - .github/skills/my-local-skill/SKILL.md local_deployed_file_hashes: @@ -141,8 +146,10 @@ deployments: | `mcp_configs` | map | no | `server_name -> resolved config dict` baseline used to detect MCP drift. | | `mcp_target_servers` | map of string lists | no | `target -> server names` for MCP entries APM successfully wrote. Reinstall and uninstall use this ownership record to remove only APM-managed entries. An explicitly empty map authorizes no cleanup. Older lockfiles without this field adopt an existing self-defined native entry only when it exactly matches the stored `mcp_configs` baseline; registry-resolved and user-edited entries remain unowned. | | `mcp_config_provenance` | map | no | `server_name -> declaring package` for transitively contributed MCP servers. Used to identify the former owner in `config-consistency` diagnostics; it never exempts a lock-only entry. | -| `lsp_servers` | list of strings | no | Names of LSP servers declared in the manifest as of the last install or update. | -| `lsp_configs` | map | no | `server_name -> resolved config dict` baseline used to detect LSP drift. | +| `lsp_servers` | list of strings | no | Names of all LSP servers in current APM-managed state, including package- and bundle-contributed servers. | +| `lsp_configs` | map | no | `server_name -> resolved config dict` retained for owner-aware lifecycle reconciliation. It is not an `apm audit` drift baseline. | +| `lsp_target_servers` | map of string lists | no | `target -> server names` for LSP entries APM successfully wrote. Reinstall and uninstall use this ownership record to remove stale executable entries without claiming legacy or user-owned content. | +| `lsp_config_provenance` | map of strings | no | `server_name -> declaration owner` using `project:.`, `package:`, or `bundle:`. Reconciliation uses this field to preserve bundle and surviving-package entries while removing departed owners. | | `local_deployed_files` | list | no | Files this project itself contributes (sources its own primitives). Reinstall reconciles these paths with the same target rules as per-dependency `deployed_files`. See [self entry](#self-entry). | | `local_deployed_file_hashes` | map | no | `path -> sha256` for `local_deployed_files`. | | `deployments` | list | no | Canonical deployment ownership rows, additive alongside the legacy `deployed_files`/`local_deployed_files` views. See [Canonical deployment rows](#canonical-deployment-rows). | @@ -174,11 +181,11 @@ and [`apm prune`](../cli/prune/#canonical-deployment-ownership). `deployments` is the canonical source APM writes to going forward. The per-dependency `deployed_files`/`deployed_file_hashes` and the top-level -`local_deployed_files`/`local_deployed_file_hashes`/`mcp_target_servers` -fields remain on disk as derived, one-cycle-compatible legacy views of the -same ledger -- older tooling that only reads the flat fields still sees a -consistent projection. Author neither view by hand; both are written by -`apm install`, `apm prune`, and related commands. +`local_deployed_files`/`local_deployed_file_hashes`/`mcp_target_servers`/ +`lsp_target_servers` fields remain on disk as derived, one-cycle-compatible +legacy views of the same ledger -- older tooling that only reads the flat +fields still sees a consistent projection. Author neither view by hand; both +are written by `apm install`, `apm prune`, and related commands. ## Per-entry fields diff --git a/docs/src/content/docs/reference/manifest-schema.md b/docs/src/content/docs/reference/manifest-schema.md index 0ad069526..09ca36aa3 100644 --- a/docs/src/content/docs/reference/manifest-schema.md +++ b/docs/src/content/docs/reference/manifest-schema.md @@ -758,7 +758,13 @@ dependencies: #### 4.3.4. What Gets Written -`apm install` writes LSP server configs to detected runtime targets. Claude Code uses `.lsp.json` at project scope or `~/.claude.json` at user scope. GitHub Copilot CLI uses `.github/lsp.json` at project scope or `~/.copilot/lsp-config.json` at user scope. See [Install LSP servers](../../consumer/install-lsp-servers/) for output formats and lifecycle details. +`apm install` writes LSP server configs to detected runtime targets. Claude +Code uses the `lspServers` section in +`.claude/skills/apm-lsp/.claude-plugin/plugin.json` at project scope or +`~/.claude.json` at user scope. GitHub Copilot CLI uses `.github/lsp.json` at +project scope or `~/.copilot/lsp-config.json` at user scope. See +[Install LSP servers](../../consumer/install-lsp-servers/) for output formats +and lifecycle details. --- diff --git a/docs/src/content/docs/reference/policy-schema.md b/docs/src/content/docs/reference/policy-schema.md index 91de5d7f2..1376312aa 100644 --- a/docs/src/content/docs/reference/policy-schema.md +++ b/docs/src/content/docs/reference/policy-schema.md @@ -69,7 +69,7 @@ possible. | `unmanaged_files` | object | see section | no | Rules over files in target directories not tracked by the lockfile. | | `security` | object | see section | no | Rules over APM's security checks (install-time content audit + external scanners; requires `external-scanners` flag). | | `registry_source` | object | see section | no | Mandate registry usage and block non-registry sources (requires `registries` flag). | -| `executables` | object | see section | no | Org ceiling for executable-primitive trust (hooks, bin, self-defined MCP, canvas). See [executables](#executables). | +| `executables` | object | see section | no | Org ceiling for executable-primitive trust (hooks, bin, self-defined MCP, LSP, canvas). See [executables](#executables). | | `bin_deploy` | object | see section | no | DEPRECATED alias folded into `executables.deny` (bin-scoped). See [bin_deploy](#bin_deploy). | Unknown top-level keys produce a warning, never an error -- so newer policy @@ -417,11 +417,11 @@ registry_source: ## executables The org ceiling for executable-primitive trust. Unifies the executable-trust -vocabulary onto one noun, `executables`, governing all four gated types: hooks, -`bin/` executables, self-defined MCP servers (`registry: false`), and canvas -extensions. The org layer is the ceiling on **deny** -- it can deny and require -fleet-wide, and recommend a vetted set, but personal or project consent can -never widen past an org deny. +vocabulary onto one noun, `executables`, governing all five gated types: hooks, +`bin/` executables, self-defined MCP servers (`registry: false`), LSP servers, +and canvas extensions. The org layer is the ceiling on **deny** -- it can deny +and require fleet-wide, and recommend a vetted set, but personal or project +consent can never widen past an org deny. | Field | Type | Default | Description | |---|---|---|---| @@ -450,10 +450,12 @@ resolved state in the `exec_status` field of `apm.lock.yaml` (one of commands that write project and personal trust, see [`apm approve`](../cli/approve/) and [`apm deny`](../cli/deny/). -There is no `enforce` mandate runtime, no cryptographic signing, and no -content-hash binding in this release: an `executables.enforce` rung is accepted -in policy but fail-safe degrades to `recommend` (allowed, still overridable by a -deny). +There is no `enforce` mandate runtime or cryptographic signing in this release: +an `executables.enforce` rung is accepted in policy but fail-safe degrades to +`recommend` (allowed, still overridable by a deny). Ordinary dependency grants +remain package-scoped. Local bundle MCP, LSP, and canvas grants use the exact +SHA-256 content key printed by `apm install`; changed bundle content requires a +new grant. ## bin_deploy diff --git a/packages/apm-guide/.apm/skills/apm-usage/commands.md b/packages/apm-guide/.apm/skills/apm-usage/commands.md index b310a49c4..16bef8583 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/commands.md +++ b/packages/apm-guide/.apm/skills/apm-usage/commands.md @@ -14,6 +14,7 @@ | `apm install [PKGS...]` | Install APM, MCP, and LSP dependencies (supports APM packages, Claude skills (SKILL.md), and plugin collections (plugin.json)); one effective target decision drives package, MCP, and LSP phases; plain/frozen installs replay locked refs and cache state, while `--update` and `--refresh` require current upstream mutable refs; a successful non-dry-run install also reconciles deployed artifacts, lockfile ownership, and merge-hook config/sidecar entries for any target dropped from `targets:` | `--update` (deprecated; prefer `apm update`) refresh refs without accepting stale bare-cache answers, `--refresh` re-fetch all deps from upstream and re-resolve all ref pins, `--force` overwrite collisions and permit deployment after critical built-in scan findings (does NOT refresh refs by itself; `apm update --force` still requires upstream truth), `--frozen` CI-safe install that fails before any durable write when `apm.lock.yaml` is missing or out of sync with `apm.yml`, including MCP config state (mutually exclusive with `--update`, package additions, and `--mcp`; use normal install to create or repair lock state, then `apm audit` for SHA integrity), `--dry-run` (no package/deployment writes; positional packages and ref changes are previewed without changing an existing `apm.yml`; a newly bootstrapped manifest and explicit targets are kept), `--verbose`, `--only [apm\|mcp]`, `--target` (comma-separated, e.g. `--target claude,cursor`; resolution chain `--target` > apm.yml `targets:` > `apm config set target ...` > auto-detect; this decision is reused by package, MCP, and LSP phases; unresolved required service work fails non-zero before manifest or package writes, and native MCP/LSP write failures also fail non-zero; `intellij` is MCP-only and writes JetBrains Copilot's user-scope config; explicit lists are exact, so `intellij,claude` writes those two MCP configs and `all,intellij` adds JetBrains to `all`; on auto-bootstrap when no `apm.yml` exists, recognized manifest target(s) are persisted to the new manifest's `targets:` field so a later bare `apm update` reuses them; `--target all` deprecated, see `apm compile --all`; use `kiro` for Kiro IDE; use `grok-build` for stable Grok Build rules, agents, commands, skills, and `AGENTS.md`; use `copilot-cowork` with `--global` after `apm experimental enable copilot-cowork`; use `grok-cloud` after `apm experimental enable grok-cloud` to deploy skills only to `.grok/skills/`; use `hermes` after `apm experimental enable hermes` to deploy skills + `AGENTS.md` and, at `--global`, MCP servers to `~/.hermes/config.yaml`), `--dev`, `-g` global (MCP deploys only to user-scope runtimes: Copilot CLI, Claude Code, Codex CLI, Gemini CLI, Antigravity CLI, Kiro, Windsurf, JetBrains Copilot, and Hermes when enabled), `--trust-transitive-mcp`, `--parallel-downloads N`, `--trust-bin` / `--no-trust-bin` (per-invocation consent for marketplace-plugin bin/ deployment: `--trust-bin` suppresses the trust-posture warning, `--no-trust-bin` skips bin/ even if policy allows; default deploys with a warning), `--allow-insecure`, `--allow-insecure-host HOSTNAME`, `--skill NAME` install named skills from a dependency that exposes selectable skills (repeatable; plugin manifests accept a leaf name or manifest path; a CLI name that matches no declared skill fails with available names; a stale persisted `skills:` pin that no longer matches an available source skill warns with the package, declared request names, and available names, and directs the user to edit `skills:` in apm.yml; persisted in apm.yml only on a successful CLI match; additive across separate installs -- a later `--skill X` adds to the existing pin (union) rather than replacing it, so previously deployed skills are never silently removed; `'*'` resets to the full bundle; drop a single skill by editing the `skills:` list in apm.yml then re-running install), `--legacy-skill-paths` restore per-client skill dirs, `--mcp NAME` add MCP entry using that same effective target decision (the shared decision applies, so `apm install --mcp NAME --target intellij` writes only JetBrains Copilot's MCP config; compilation target policy applies to every explicitly selected target; `apm install -g --mcp NAME` writes user-scope and bypasses the project-scope gate by design), `--transport`, `--url`, `--env KEY=VAL`, `--header KEY=VAL`, `--mcp-version`, `--registry URL` custom MCP registry, `--root DIR` redirect writes (`apm_modules/`, lockfile, `.gitignore`, integrated harness files) under DIR while `apm.yml`/`.apm/`/local deps resolve from `$PWD` (mirrors `pip install --target`; created if missing; not valid with `-g`/`--global`, which exits 2). Explicit plugin component paths must resolve inside the plugin root; missing declarations fail before deployment and lockfile commit. | | `apm targets` | Show resolved deployment targets for the current project (Click group; reads filesystem signals; works with or without `apm.yml`) | `--all` also include the `agent-skills` meta-target (only meaningful with `--json`), `--json` machine-readable output. No provenance line is printed (the table is the provenance). | | `apm uninstall PKGS...` | Remove packages; identifier selection is atomic. Accepts `owner/repo`, `name@marketplace`, exact declared local paths, or portable `_local/` keys for direct local declarations with matching lock metadata. A missing or ambiguous identifier exits nonzero before scripts or APM writes. MCP cleanup uses recorded target ownership, attempts every owning runtime, and exits nonzero if any target fails. | `--dry-run`, `-g` global | +| `apm uninstall PKGS...` | Remove packages and reconcile their tracked files, MCP servers, and LSP servers; identifier selection is atomic. Accepts `owner/repo`, `name@marketplace`, exact declared local paths, or portable `_local/` keys for direct local declarations with matching lock metadata. A missing or ambiguous identifier exits nonzero before scripts or APM writes. If safe LSP cleanup fails after package removal, the command exits nonzero and preserves the conflicting config; repair it, then run `apm install` (or `apm install --global` for user scope). | `--dry-run`, `-g` global | | `apm prune` | Remove installed packages absent from the manifest and lockfile-resolved graph; reconcile stale dependency/deployment ownership after interrupted runs without deleting files based only on ghost metadata or dropping shared URI deployments | `--dry-run` previews package removal and ownership repair without mutation | | `apm deps list` | List manifest- and lockfile-resolved packages; ignore parent-owned embedded manifests. Direct locked local packages use actionable `_local/` keys without absolute paths; transitives are removed through their parent. | `-g` global, `--all` both scopes, `--insecure` | | `apm deps tree` | Show the complete lockfile-resolved tree at any depth; mark repeated ancestors as circular | -- | @@ -104,7 +105,7 @@ entry migrates only provenance-owned servers from the obsolete data path. [i] Targets: claude, copilot (source: auto-detect from CLAUDE.md, .github/copilot-instructions.md) ``` -Suppress with `--quiet`. Add `--verbose` to also print a `[>] Scanned: ...` line listing every signal probed. +Add `--verbose` to also print a `[>] Scanned: ...` line listing every signal probed. If no `--target`, no `targets:` in `apm.yml`, and no harness signal is present, `apm install` exits 2 with a teaching message instead of silently defaulting to copilot. Run `apm targets` to inspect what APM detects in the current directory; use it for discovery, scripting (`--json`), and debugging unexpected detection. @@ -114,6 +115,13 @@ writing the direct MCP manifest entry or deploying packages. Native service config write failures also exit non-zero with an actionable path/permissions diagnostic. +For project LSP installs, `apm install --target claude` writes the +`lspServers` section in +`.claude/skills/apm-lsp/.claude-plugin/plugin.json`, which Claude Code +discovers as a skills-directory plugin. Global Claude LSP installs use the +same personal-scope plugin shape at +`~/.claude/skills/apm-lsp/.claude-plugin/plugin.json`. + For MCP, `--target copilot` writes `.github/mcp.json` at project scope and `$COPILOT_HOME/mcp-config.json` with `--global` (or `~/.copilot/mcp-config.json` when `COPILOT_HOME` is unset). `--target vscode` @@ -401,10 +409,10 @@ Experimental flags MUST NOT gate security-critical behaviour (content scanning, `apm config set mcp-registry-url https://mcp.internal.example.com` persists a private MCP registry URL so users do not need to export `MCP_REGISTRY_URL` every session. Accepts `http://` or `https://` URLs; all other schemes are rejected. A configured `http://` endpoint also requires `MCP_REGISTRY_ALLOW_HTTP=1` when used; an explicit `--registry http://...` flag is the per-invocation opt-in. Resolution order: `--registry ` flag on `apm mcp install` / `apm install --mcp` > `MCP_REGISTRY_URL` env var > `mcp-registry-url` in `~/.apm/config.json` > built-in public default. When the config layer is active, `apm mcp search` prints a `Registry (config): ` diagnostic. `apm config unset mcp-registry-url` removes the persisted URL. -`apm approve [PACKAGE_REF...]` grants a package permission to deploy executable primitives (hooks, `bin/`, self-defined MCP servers, canvas extensions). By default it writes the project `apm.yml` `executables.allow` block -- committed to source control, so the whole team inherits the trust decision. `apm approve --user` records a personal grant in `~/.apm/config.json` instead (machine-local, never committed, lowest authority -- it can only narrow trust). +`apm approve [PACKAGE_REF...]` grants a package permission to deploy executable primitives (hooks, `bin/`, self-defined MCP servers, LSP servers, canvas extensions). By default it writes the project `apm.yml` `executables.allow` block -- committed to source control, so the whole team inherits the trust decision. `apm approve --user` records a personal grant in `~/.apm/config.json` instead (machine-local, never committed, lower authority than org or project denies). The project `apm.yml` must carry an `executables:` block (even empty `{}`) to enable the gate; without it, all executables are allowed unconditionally. Flags: `--pending` lists packages with unapproved executables; `--all` approves every currently blocked package; `--recommended` bulk-accepts the org `executables.recommend` set; `--list` shows the fleet-level effective trust decision and deciding layer per installed package. `apm deny [OPTIONS] PACKAGES...` runs from an APM project with `apm.yml` and writes a block to the project `executables.deny` (or `~/.apm/config.json` with `--user`); deny always wins. When no installed executable declaration is found, APM records all supported executable types, so denying a not-yet-installed package is allowed as a pre-emptive block. `apm policy explain ` (a subcommand of the `apm policy` group, sibling to `apm policy status`) prints the effective decision for a package: allowed or blocked per type, the deciding policy layer, and any shadowed (overridden) layers. `apm doctor` adds a fleet-level executable-trust drift check that flags packages allowed locally but denied by org policy. -The legacy top-level `allowExecutables:` block is a deprecated alias for `executables.allow`, read for one minor cycle and migrated on the next approve/deny write; the standalone `~/.apm/approvals.yml` is removed and migrated into `~/.apm/config.json` on first read. Grant keys are package-scoped in v1: `owner/repo` and `owner/repo#version` both match the package regardless of installed version. In CI, pre-approve packages by committing them to `executables.allow`; untrusted required executables fail `apm audit` with `required-executable-untrusted`. +The legacy top-level `allowExecutables:` block is a deprecated alias for `executables.allow`, read for one minor cycle and migrated on the next approve/deny write; the standalone `~/.apm/approvals.yml` is removed and migrated into `~/.apm/config.json` on first read. Grant keys for ordinary dependencies are package-scoped in v1: `owner/repo` and `owner/repo#version` both match the package regardless of installed version. Local bundle grants are content-bound: copy the full `name#version@sha256:` key printed by `apm install`; changed bundle bytes need a new grant. In CI, pre-approve packages by committing them to `executables.allow`; untrusted required executables fail `apm audit` with `required-executable-untrusted`. diff --git a/packages/apm-guide/.apm/skills/apm-usage/dependencies.md b/packages/apm-guide/.apm/skills/apm-usage/dependencies.md index e588cddcf..a79a191f6 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/dependencies.md +++ b/packages/apm-guide/.apm/skills/apm-usage/dependencies.md @@ -563,20 +563,27 @@ Optional fields: `args`, `transport`, `env`, `initializationOptions`, `settings`, `workspaceFolder`, `startupTimeout`, `shutdownTimeout`, `restartOnCrash`, `maxRestarts`. -`apm install` writes LSP config to the detected runtime targets: -Claude Code uses `.lsp.json` or `~/.claude.json`, and GitHub Copilot CLI -uses `.github/lsp.json` or `~/.copilot/lsp-config.json`. Copilot CLI -uses `fileExtensions` on disk; manifests continue to use -`extensionToLanguage`. Plugin `.lsp.json` files may use either a flat -server map or a `{ "lspServers": { ... } }` envelope. For Copilot-dialect -plugin input, APM accepts `fileExtensions` as an alias for -`extensionToLanguage` and `warmupTimeoutMs` as an alias for -`startupTimeout`; a non-null canonical value wins when both are supplied, -while a null canonical value falls back to its alias. APM ignores the -unsupported Copilot `cwd` field and warns that the consumer runtime chooses -the working directory. Copilot output uses `fileExtensions` and -`warmupTimeoutMs`; manifests and lockfiles retain `extensionToLanguage` and -`startupTimeout`. +`apm install` writes LSP config to the detected runtime targets. Claude Code +project installs use the `lspServers` section in +`.claude/skills/apm-lsp/.claude-plugin/plugin.json`; global installs use +`~/.claude/skills/apm-lsp/.claude-plugin/plugin.json`. GitHub Copilot CLI uses +`.github/lsp.json` or `~/.copilot/lsp-config.json`. Copilot CLI uses +`fileExtensions` on disk; +manifests continue to use `extensionToLanguage`. A dependency package's source +`.lsp.json` may use either a flat server map or a +`{ "lspServers": { ... } }` envelope; it is distinct from the Claude project +plugin manifest that APM generates. Dependency-provided LSP commands require +executable approval for the declaring package when a project or org +`executables` block enables the gate; the compatibility default permits them +when no layer opts in. For Copilot-dialect plugin input, APM accepts +`fileExtensions` as an alias for `extensionToLanguage` and `warmupTimeoutMs` as +an alias for `startupTimeout`; a non-null canonical value wins when both are +supplied, while a null canonical value falls back to its alias. APM ignores the +unsupported Copilot `cwd` field and warns that the consumer runtime chooses the +working directory. Copilot output uses `fileExtensions` and `warmupTimeoutMs`; +manifests and lockfiles retain `extensionToLanguage` and `startupTimeout`. APM +records target-scoped LSP ownership in the lockfile so target changes and +package uninstall revoke only entries it wrote. ## Version pinning diff --git a/packages/apm-guide/.apm/skills/apm-usage/governance.md b/packages/apm-guide/.apm/skills/apm-usage/governance.md index d2dc1e9f9..f28e0df78 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/governance.md +++ b/packages/apm-guide/.apm/skills/apm-usage/governance.md @@ -175,10 +175,10 @@ Notes: ## Executable trust governance Issue #1873 unifies executable-primitive trust (hooks, `bin/` executables, -self-defined MCP servers, canvas extensions) onto one noun, `executables`, -across three layers. The org policy is the **ceiling on deny**: it can deny and -require fleet-wide and recommend a vetted set, but personal or project consent -can never widen past an org deny. +self-defined MCP servers, LSP servers, canvas extensions) onto one noun, +`executables`, across three layers. The org policy is the **ceiling on deny**: +it can deny and require fleet-wide and recommend a vetted set, but personal or +project consent can never widen past an org deny. ```yaml # .github/apm-policy.yml @@ -229,14 +229,17 @@ package PRESENCE, not materialized files. A separate audit signal, `required-executable-untrusted`, hard-fails CI when a required package's executables are untrusted (denied or gated). -There is no `enforce` mandate runtime, no cryptographic signing, and no -content-hash binding in this release: an org `executables.enforce` rung is -accepted but fail-safe degrades to `recommend` (allowed, still overridable by a -deny). Inspect the deciding layer for one package with `apm policy explain -`, and surface fleet-wide layer conflicts (packages allowed locally but -denied by org policy) with `apm doctor`. The same doctor row reports a malformed -project executable-trust configuration under either `executables` or the -deprecated `allowExecutables` key and names the configuration to fix. +There is no `enforce` mandate runtime or package signing in this release: an org +`executables.enforce` rung is accepted but fail-safe degrades to `recommend` +(allowed, still overridable by a deny). Local bundle LSP, MCP, and canvas +approvals use a narrower content-bound key: when one is skipped, APM prints an +exact `executables.allow` entry containing that bundle's SHA-256 digest. A +different bundle that claims the same package name cannot inherit the grant. Inspect the +deciding layer for one package with `apm policy explain `, and surface +fleet-wide layer conflicts (packages allowed locally but denied by org policy) +with `apm doctor`. The same doctor row reports a malformed project +executable-trust configuration under either `executables` or the deprecated +`allowExecutables` key and names the configuration to fix. ## Plugin bin/ deployment governance (deprecated alias) @@ -289,8 +292,10 @@ Because a canvas from a dependency is arbitrary executable code, APM blocks dependency-provided canvases when the project opts in to the executable gate: the project must add an `executables:` block to `apm.yml` and run `apm approve ` to deploy it. A first-party canvas in the root package being -installed deploys once the flag is on; dependency canvases always require -explicit approval. +installed deploys once the flag is on. With `--global`, dependency-provided +canvases always require explicit approval. Local bundle canvases use the exact +`name#version@sha256:` key printed by `apm install`, so changed bundle +bytes require renewed consent. By default `apm approve` records the grant in the project `apm.yml` `executables.allow` block (committed, shared with the team); `apm approve --user` @@ -309,12 +314,12 @@ global canvas. The trust gate is enforced on every install path -- normal install and offline bundle install (`apm install `) -- so a vendored bundle cannot smuggle -an executable canvas past trust. Canvas trust is unified with the `executables` -default-deny gate (hooks, bin, mcp, canvas); approve once and all four -executable types are governed consistently. The org `executables:` policy block -governs canvas trust alongside the other types (`deny_all`, `deny`, `require`, -`recommend`); a canvas-only policy knob is not part of this experimental -release. +an executable canvas past trust. Once an `executables:` block enables the +default-deny gate, canvas trust is unified with hook, bin, MCP, and LSP +execution; approve once and all five executable types are governed consistently. +The org `executables:` policy block governs canvas trust alongside +the other types (`deny_all`, `deny`, `require`, `recommend`); a canvas-only +policy knob is not part of this experimental release. ## Local content governance diff --git a/scripts/architecture_linter/checks/install_base_integrator_and_contraction.py b/scripts/architecture_linter/checks/install_base_integrator_and_contraction.py index cc7890dfd..81855ea76 100644 --- a/scripts/architecture_linter/checks/install_base_integrator_and_contraction.py +++ b/scripts/architecture_linter/checks/install_base_integrator_and_contraction.py @@ -289,6 +289,7 @@ def check_target_file_contraction(provider: FactsProvider) -> tuple[Violation, . "local_deployed_files", "local_deployed_file_hashes", "mcp_target_servers", + "lsp_target_servers", } ) diff --git a/scripts/architecture_linter/checks/install_deployment_analyzers.py b/scripts/architecture_linter/checks/install_deployment_analyzers.py index 421848c8a..9aac19e74 100644 --- a/scripts/architecture_linter/checks/install_deployment_analyzers.py +++ b/scripts/architecture_linter/checks/install_deployment_analyzers.py @@ -36,6 +36,14 @@ check_mcp_ownership_migration, check_uninstall_reachability, ) +from scripts.architecture_linter.checks.install_lsp_plugin import ( + GUARD_EXECUTABLE_TRUST, + GUARD_LSP_LIFECYCLE, + GUARD_LSP_TARGET_CONTRACT, + check_executable_trust_context, + check_lsp_lifecycle, + check_lsp_target_contract, +) from scripts.architecture_linter.checks.install_package_target_authorization import ( _GUARD_PACKAGE_TARGET, check_package_target_authorization, @@ -120,6 +128,21 @@ "File-level deploy/sync/cleanup stays owned by BaseIntegrator.", check_base_integrator, ), + _rule( + GUARD_EXECUTABLE_TRUST, + "Install and update consume one effective executable-trust owner.", + check_executable_trust_context, + ), + _rule( + GUARD_LSP_TARGET_CONTRACT, + "LSP target shape and deployment paths route through LSPIntegrator.", + check_lsp_target_contract, + ), + _rule( + GUARD_LSP_LIFECYCLE, + "LSP collection and reconciliation route through install/lsp/integration.py.", + check_lsp_lifecycle, + ), _rule( _GUARD_UNINSTALL_REACHABILITY, "Post-uninstall dependency reachability routes through deps/reachability.py.", diff --git a/scripts/architecture_linter/checks/install_lsp_plugin.py b/scripts/architecture_linter/checks/install_lsp_plugin.py new file mode 100644 index 000000000..6d00e0208 --- /dev/null +++ b/scripts/architecture_linter/checks/install_lsp_plugin.py @@ -0,0 +1,180 @@ +"""Claude LSP plugin and executable-trust architecture guards.""" + +from __future__ import annotations + +from scripts.architecture_linter.checks.install_deployment_shared import ( + _facts_for, + _present, + _python_paths, + _summary, +) +from scripts.architecture_linter.facts import FactsProvider +from scripts.architecture_linter.models import Violation + +GUARD_EXECUTABLE_TRUST = "install-deployment-executable-trust-context" +GUARD_LSP_TARGET_CONTRACT = "install-deployment-lsp-target-contract" +GUARD_LSP_LIFECYCLE = "install-deployment-lsp-lifecycle" + +_EXECUTABLES = "src/apm_cli/security/executables.py" +_APPROVE_COMMAND = "src/apm_cli/commands/approve.py" +_INSTALL_COMMAND = "src/apm_cli/commands/install.py" +_INSTALL_TEMPLATE = "src/apm_cli/install/template.py" +_LSP_INTEGRATOR = "src/apm_cli/integration/lsp_integrator.py" +_LSP_PIPELINE = "src/apm_cli/install/lsp/integration.py" +_LOCAL_BUNDLE = "src/apm_cli/install/local_bundle_handler.py" +_INSTALL_SERVICES = "src/apm_cli/install/services.py" +_SKILL_SUPPORT = "src/apm_cli/integration/skill_support.py" + + +def _missing_tokens( + provider: FactsProvider, + rule_id: str, + required: dict[str, tuple[str, ...]], +) -> tuple[Violation, ...]: + findings: list[Violation] = [] + for path, tokens in required.items(): + facts, failures = _facts_for(provider, path, rule_id) + if failures: + findings.extend(failures) + continue + missing = [token for token in tokens if not _present(facts, token)] + if missing: + findings.append( + _summary( + rule_id, + path, + "Canonical routing tokens are missing: " + ", ".join(missing), + ) + ) + return tuple(findings) + + +def check_executable_trust_context(provider: FactsProvider) -> tuple[Violation, ...]: + """Install and update must consume one effective executable-trust owner.""" + return _missing_tokens( + provider, + GUARD_EXECUTABLE_TRUST, + { + _EXECUTABLES: ( + "def exec_trust_context_for_project(", + "def locked_dependency_approval_keys(", + "def local_bundle_approval_key(", + 'owner = getattr(dependency, "resolved_by", None)', + 'approval_keys = getattr(dependency, "approval_keys", ())', + ), + _APPROVE_COMMAND: ("approval_identity=locked.get_unique_key()",), + _INSTALL_COMMAND: ("effective_bundle_allow_map as _effective_bundle_allow_map",), + _INSTALL_TEMPLATE: ("trust_ctx = exec_trust_context_for_project(",), + _LSP_PIPELINE: ( + "effective_allow_executables = effective_exec_map_for_project(", + "if not effective_allow_resolved:", + ), + _LOCAL_BUNDLE: ( + "bundle_approval_key = local_bundle_approval_key(", + "approval_key=bundle_approval_key,", + ), + _INSTALL_SERVICES: ( + "is_package_approved(allow_executables, approval_key, EXEC_TYPE_CANVAS)", + ), + }, + ) + + +def check_lsp_target_contract(provider: FactsProvider) -> tuple[Violation, ...]: + """LSP target shape and deployment paths must route through LSPIntegrator.""" + return _missing_tokens( + provider, + GUARD_LSP_TARGET_CONTRACT, + { + _LSP_INTEGRATOR: ( + "def reserved_project_skill_names(", + "def _prepare_target_config(", + "prepared_targets.append((runtime, spec, prepared))", + "BaseIntegrator.resolve_deploy_path(relative_path, project_root)", + "allowed_prefixes=(relative_path,)", + "locked_dependency_approval_keys(locked_dependency)", + "approval_keys=approval_keys", + ), + _SKILL_SUPPORT: ( + "LSPIntegrator.reserved_project_skill_names(skills_dir, project_root)", + ), + }, + ) + + +def check_lsp_lifecycle(provider: FactsProvider) -> tuple[Violation, ...]: + """LSP collection and reconciliation must route through the pipeline owner.""" + findings = list( + _missing_tokens( + provider, + GUARD_LSP_LIFECYCLE, + { + _LSP_PIPELINE: ( + "transitive_lsp = filter_lsp_by_allow_executables(", + "lsp_deps = LSPIntegrator.deduplicate(lsp_deps + transitive_lsp)", + "def reconcile_lsp_after_uninstall(", + "lsp_target_servers=new_targets", + "managed_target_servers=old_lsp_targets", + ), + _LOCAL_BUNDLE: ( + "def effective_bundle_allow_map(", + "return effective_exec_map_for_project(", + "def _wire_bundle_lsp_servers(", + "force=force,", + ), + }, + ) + ) + pipeline, failures = _facts_for(provider, _LSP_PIPELINE, GUARD_LSP_LIFECYCLE) + if failures: + return tuple([*findings, *failures]) + lines = getattr(pipeline, "lines", ()) + filter_line = next( + ( + index + for index, line in enumerate(lines) + if "transitive_lsp = filter_lsp_by_allow_executables(" in line + ), + None, + ) + dedup_line = next( + ( + index + for index, line in enumerate(lines) + if "lsp_deps = LSPIntegrator.deduplicate(lsp_deps + transitive_lsp)" in line + ), + None, + ) + if filter_line is not None and dedup_line is not None and filter_line >= dedup_line: + findings.append( + _summary( + GUARD_LSP_LIFECYCLE, + _LSP_PIPELINE, + "Transitive LSP trust filtering must run before first-wins deduplication", + ) + ) + lifecycle_calls = ( + "LSPIntegrator.collect_transitive(", + "LSPIntegrator.resolve_target_runtimes(", + "LSPIntegrator.install(", + "LSPIntegrator.remove_stale(", + "LSPIntegrator.update_lockfile(", + ) + for path in _python_paths(provider, "src/apm_cli/"): + if path in {_LSP_INTEGRATOR, _LSP_PIPELINE}: + continue + facts, failures = _facts_for(provider, path, GUARD_LSP_LIFECYCLE) + if failures: + findings.extend(failures) + continue + direct_calls = [token for token in lifecycle_calls if _present(facts, token)] + if direct_calls: + findings.append( + _summary( + GUARD_LSP_LIFECYCLE, + path, + "LSP lifecycle calls must route through install/lsp/integration.py: " + + ", ".join(direct_calls), + ) + ) + return tuple(findings) diff --git a/scripts/architecture_linter/checks/marketplace_legacy.py b/scripts/architecture_linter/checks/marketplace_legacy.py index 2765fc425..d332c5bcf 100644 --- a/scripts/architecture_linter/checks/marketplace_legacy.py +++ b/scripts/architecture_linter/checks/marketplace_legacy.py @@ -238,9 +238,7 @@ def _python_paths(provider: FactsProvider, prefix: str) -> tuple[str, ...]: _LOCAL_BUNDLE_IMPORT = re.compile( r"from \.\.install\.local_bundle_handler import install_local_bundle" ) -_EXECUTABLE_TRUST_IMPORT = re.compile( - r"from \.\.security\.executables import read_bundle_allow_executables" -) +_EXECUTABLE_TRUST_IMPORT = re.compile(r"_allow_execs_for_bundle = _effective_bundle_allow_map") def _check_format_owner( diff --git a/src/apm_cli/commands/approve.py b/src/apm_cli/commands/approve.py index 11aeba41e..d3e52cd5b 100644 --- a/src/apm_cli/commands/approve.py +++ b/src/apm_cli/commands/approve.py @@ -139,7 +139,7 @@ def approve_cmd( list_decisions: bool, user_scope: bool, ) -> None: - """Approve executable primitives (hooks, MCP, bin, canvas) for packages. + """Approve executable primitives (hooks, MCP, LSP, bin, canvas) for packages. By default writes to the project ``apm.yml`` ``executables.allow`` block (committed). Use ``--user`` to record a personal grant in @@ -221,7 +221,9 @@ def deny_cmd(packages: tuple[str, ...], user_scope: bool) -> None: break if decl is None: # Allow denying a package that is not (or no longer) installed. - deny[pkg] = {t: True for t in ("hooks", "mcp", "bin", "canvas")} + from ..security.executables import ALL_EXEC_TYPES + + deny[pkg] = dict.fromkeys(ALL_EXEC_TYPES, True) allow.pop(_find_matching_key(allow, pkg) or pkg, None) _rich_info(f"Denied {pkg} (all executable types)", symbol="info") changed += 1 @@ -485,6 +487,57 @@ def scan_installed_executable_packages(manifest: Path) -> list: if not apm_modules.is_dir(): return results + from ..deps.lockfile import LockFile, get_lockfile_path + + lockfile = LockFile.read(get_lockfile_path(manifest.parent)) + if lockfile is not None: + from ..deps.path_anchoring import LocalResolutionError, resolve_local_dep_dirs + from ..utils.yaml_io import load_yaml + + try: + local_dependency_dirs = resolve_local_dep_dirs(lockfile, manifest.parent) + except LocalResolutionError as exc: + raise click.ClickException( + "Cannot inspect installed packages because apm.lock.yaml has " + f"invalid local dependency ancestry: {exc}. Run 'apm install' " + "to regenerate the lockfile, then retry." + ) from exc + for locked in lockfile.get_package_dependencies(): + try: + package_dir = ( + local_dependency_dirs[locked.get_unique_key()] + if locked.source == "local" + else locked.to_dependency_ref().get_install_path(apm_modules) + ) + except (OSError, ValueError): + continue + if not package_dir.is_dir(): + continue + name = locked.name or package_dir.name + version = str(locked.version or "") + package_manifest = package_dir / "apm.yml" + manifest_data = None + if package_manifest.is_symlink(): + manifest_data = {} + elif package_manifest.is_file(): + try: + manifest_data = load_yaml(package_manifest) + if isinstance(manifest_data, dict): + name = manifest_data.get("name", name) + version = str(manifest_data.get("version", version)) + except Exception: + manifest_data = {} + declaration = scan_package_executables( + package_dir, + name, + version, + approval_identity=locked.get_unique_key(), + manifest_data=manifest_data, + ) + if declaration.has_executables: + results.append(declaration) + return results + def _scan_dir(base: Path) -> None: for pkg_dir in sorted(base.iterdir()): if not pkg_dir.is_dir() or pkg_dir.name.startswith("."): @@ -496,18 +549,26 @@ def _scan_dir(base: Path) -> None: pkg_yml = pkg_dir / "apm.yml" name = pkg_dir.name version = "" - if pkg_yml.is_file(): + manifest_data = None + if pkg_yml.is_symlink(): + manifest_data = {} + elif pkg_yml.is_file(): try: from ..utils.yaml_io import load_yaml - data = load_yaml(pkg_yml) - if isinstance(data, dict): - name = data.get("name", name) - version = str(data.get("version", "")) + manifest_data = load_yaml(pkg_yml) + if isinstance(manifest_data, dict): + name = manifest_data.get("name", name) + version = str(manifest_data.get("version", "")) except Exception: - pass - - decl = scan_package_executables(pkg_dir, name, version) + manifest_data = {} + + decl = scan_package_executables( + pkg_dir, + name, + version, + manifest_data=manifest_data, + ) if decl.has_executables: results.append(decl) diff --git a/src/apm_cli/commands/install.py b/src/apm_cli/commands/install.py index 5f0daa7d6..0848a8e4b 100644 --- a/src/apm_cli/commands/install.py +++ b/src/apm_cli/commands/install.py @@ -203,6 +203,8 @@ class InstallContext: install_result: InstallResult | None = None target_decision: "EffectiveTargetDecision | None" = None trust_bin: bool | None = None + exec_allow_map: builtins.dict[str, builtins.dict[str, bool]] | None = None + exec_allow_resolved: bool = False # APM Dependencies (conditional import for graceful degradation) @@ -1228,7 +1230,6 @@ def install( # noqa: C901, PLR0913 "--frozen and --update are mutually exclusive. " "Use 'apm update' to refresh refs, then 'apm install --frozen' in CI." ) - # The root redirect restores cwd in the command's existing finally block. if root and global_: raise click.UsageError("--root is not valid with --global (user scope)") from ..core.install_audit import resolve_audit_override_from_cli @@ -1243,24 +1244,18 @@ def install( # noqa: C901, PLR0913 InstallService.reject_missing_frozen_root(frozen, root) except FrozenInstallError as exc: raise click.ClickException(str(exc)) from exc + _source_root = Path.cwd() _root_redirect = install_root_redirect(root, dry_run=dry_run) _root_redirect.__enter__() try: - # Create structured logger for install output early so exception - # handlers can always reference it (avoids UnboundLocalError if - # scope initialisation below throws). is_partial = bool(packages) logger = InstallLogger(verbose=verbose, dry_run=dry_run, partial=is_partial) - # Resolve --legacy-skill-paths: CLI flag wins, then env var fallback. if not legacy_skill_paths: from ..integration.targets import should_use_legacy_skill_paths legacy_skill_paths = should_use_legacy_skill_paths() - # ---------------------------------------------------------------- - # Local bundles bypass dependency resolution and do not mutate apm.yml. - # ---------------------------------------------------------------- if len(packages) == 1 and not mcp_name and (_probe := Path(packages[0])).exists(): from ..bundle.local_bundle import detect_local_bundle as _detect_lb @@ -1272,12 +1267,17 @@ def install( # noqa: C901, PLR0913 raise click.UsageError(f"Bundle security check failed: {exc}") from exc if _bundle_info is not None: enforce_agent_plugin_deployment_boundary(bundle_info=_bundle_info) + from ..install.local_bundle_handler import ( + effective_bundle_allow_map as _effective_bundle_allow_map, + ) from ..install.local_bundle_handler import install_local_bundle as _install_lb - # allowExecutables for bundle install gate. - from ..security.executables import read_bundle_allow_executables as _rbae - - _allow_execs_for_bundle = _rbae(Path(root or ".") / "apm.yml", logger) + _bundle_project_root = _source_root + _allow_execs_for_bundle = _effective_bundle_allow_map( + _bundle_project_root, + no_policy=no_policy, + logger=logger, + ) _install_lb( bundle_info=_bundle_info, bundle_arg=packages[0], @@ -1352,11 +1352,9 @@ def install( # noqa: C901, PLR0913 if verbose: os.environ["APM_VERBOSE"] = "1" - # ---------------------------------------------------------------- # --mcp branch (W3): when --mcp is set, route to the dedicated # MCP-add path. We compute the post-`--` argv here BEFORE Click's # silent handling: see _split_argv_at_double_dash(). - # ---------------------------------------------------------------- _, command_argv = _split_argv_at_double_dash(_get_invocation_argv()) # `packages` from Click already includes the post-`--` items; the # pre-`--` portion is what the user typed as positional packages. @@ -1918,6 +1916,8 @@ def _install_apm_packages(ctx, outcome): apm_count = install_result.installed_count apm_diagnostics = install_result.diagnostics ctx.target_decision = install_result.target_decision + ctx.exec_allow_map = install_result.exec_allow_map + ctx.exec_allow_resolved = install_result.exec_allow_resolved if install_result.disposition not in { InstallDisposition.SUCCESS, InstallDisposition.PARTIAL_SUCCESS, diff --git a/src/apm_cli/commands/uninstall/cli.py b/src/apm_cli/commands/uninstall/cli.py index 6a59a5011..b4991dcea 100644 --- a/src/apm_cli/commands/uninstall/cli.py +++ b/src/apm_cli/commands/uninstall/cli.py @@ -4,6 +4,7 @@ import contextlib import sys import traceback +from typing import Any import click @@ -43,6 +44,53 @@ def _prepare_dependency_sections(data: dict) -> tuple[bool, list, list, list]: return had_dev_section, prod_deps, dev_deps, [*prod_deps, *dev_deps] +def _cleanup_stale_lsp( + *, + apm_package: Any, + lockfile: Any, + lockfile_path: Any, + modules_dir: Any, + deploy_root: Any, + user_scope: bool, + logger: Any, +) -> tuple[bool, Exception | None]: + """Reconcile LSP state after uninstall and render an actionable failure.""" + try: + from ...install.lsp.integration import reconcile_lsp_after_uninstall + + updated = reconcile_lsp_after_uninstall( + apm_package=apm_package, + lockfile=lockfile, + lock_path=lockfile_path, + modules_dir=modules_dir, + project_root=deploy_root, + user_scope=user_scope, + logger=logger, + ) + return updated, None + except Exception as cleanup_error: + recovery_command = "apm install --global" if user_scope else "apm install" + logger.error( + "Uninstall incomplete: package removal completed, but LSP cleanup " + f"failed: {cleanup_error}. Fix the LSP config path, then run " + f"'{recovery_command}' to reconcile stale entries." + ) + logger.verbose_detail(traceback.format_exc().rstrip()) + return False, cleanup_error + + +def _abort_if_retained_target_cleanup_paths(retained_cleanup_paths: set[Any], logger: Any) -> None: + """Stop uninstall before package state mutates when owned target files remain.""" + if retained_cleanup_paths: + logger.error( + "Uninstall could not remove tracked target files; package state was preserved." + ) + for path in sorted(retained_cleanup_paths): + logger.error(f" - {path}") + logger.error("Resolve or remove the listed files, then retry uninstall.") + sys.exit(1) + + @click.command( help="Remove packages using manifest entries or direct locked keys from 'apm deps list'" ) @@ -93,6 +141,7 @@ def uninstall(ctx, packages, dry_run, verbose, global_): logger = CommandLogger("uninstall", verbose=verbose, dry_run=dry_run) staged_local_refreshes = {} + apm_package = None registration_token = _publish_native_registration(deploy_root, scope, manifest_path) try: # Check if apm.yml exists @@ -289,14 +338,7 @@ def uninstall(ctx, packages, dry_run, verbose, global_): user_scope=scope is InstallScope.USER, logger=logger, ) - if retained_cleanup_paths: - logger.error( - "Uninstall could not remove tracked target files; package state was preserved." - ) - for path in sorted(retained_cleanup_paths): - logger.error(f" - {path}") - logger.error("Resolve or remove the listed files, then retry uninstall.") - sys.exit(1) + _abort_if_retained_target_cleanup_paths(retained_cleanup_paths, logger) # Step 4: Remove from apm.yml for package in packages_to_remove: @@ -485,6 +527,17 @@ def uninstall(ctx, packages, dry_run, verbose, global_): ) logger.verbose_detail(traceback.format_exc().rstrip()) + lsp_lock_updated, lsp_cleanup_error = _cleanup_stale_lsp( + apm_package=apm_package, + lockfile=lockfile, + lockfile_path=lockfile_path, + modules_dir=modules_dir, + deploy_root=deploy_root, + user_scope=scope is InstallScope.USER, + logger=logger, + ) + lockfile_updated = lsp_lock_updated or lockfile_updated + if lockfile and lockfile_updated and lockfile_ready: try: from .lockfile_state import lockfile_has_persisted_state @@ -512,7 +565,7 @@ def uninstall(ctx, packages, dry_run, verbose, global_): summary_lines = [f"Removed {len(packages_to_remove)} package(s) from apm.yml"] if removed_from_modules > 0: summary_lines.append(f"Removed {removed_from_modules} package(s) from apm_modules/") - if mcp_cleanup_error is None: + if mcp_cleanup_error is None and lsp_cleanup_error is None: logger.success("Uninstall complete: " + ", ".join(summary_lines)) # Fire post-uninstall lifecycle scripts @@ -525,7 +578,7 @@ def uninstall(ctx, packages, dry_run, verbose, global_): verbose=verbose, deploy_root=deploy_root, ) - if mcp_cleanup_error is not None: + if mcp_cleanup_error is not None or lsp_cleanup_error is not None: sys.exit(1) except Exception as e: diff --git a/src/apm_cli/commands/update.py b/src/apm_cli/commands/update.py index b411e4122..90661a19c 100644 --- a/src/apm_cli/commands/update.py +++ b/src/apm_cli/commands/update.py @@ -230,6 +230,9 @@ def _run_mcp_lsp_integration( diagnostics: Any, logger: InstallLogger, verbose: bool, + effective_allow_executables: dict[str, dict[str, bool]] | None = None, + effective_allow_resolved: bool = False, + force: bool = False, ) -> None: """Reconcile MCP and LSP servers against the current apm.yml. @@ -310,6 +313,9 @@ def _run_mcp_lsp_integration( target_context=(mcp_apm_config, effective_target, scope), target_decision=target_decision, fail_on_write_error=True, + effective_allow_executables=effective_allow_executables, + effective_allow_resolved=effective_allow_resolved, + force=force, ) @@ -321,6 +327,7 @@ def _handle_service_only_update( dry_run: bool, logger: InstallLogger, verbose: bool, + force: bool, ) -> bool: """Reconcile service-only manifests and return whether update is complete.""" if apm_package.has_any_apm_dependencies(): @@ -362,6 +369,7 @@ def _handle_service_only_update( diagnostics=None, logger=logger, verbose=verbose, + force=force, ) except RequiredIntegrationError as exc: logger.error(str(exc)) @@ -609,6 +617,7 @@ def _run_dep_update( dry_run=dry_run, logger=logger, verbose=verbose, + force=force, ): return @@ -834,6 +843,9 @@ def _plan_callback(plan: UpdatePlan) -> bool: diagnostics=getattr(result, "diagnostics", None), logger=logger, verbose=verbose, + effective_allow_executables=getattr(result, "exec_allow_map", None), + effective_allow_resolved=getattr(result, "exec_allow_resolved", False), + force=force, ) except RequiredIntegrationError as e: logger.error(str(e)) diff --git a/src/apm_cli/core/deployment_ledger.py b/src/apm_cli/core/deployment_ledger.py index 47bc6d3f1..2d9809b16 100644 --- a/src/apm_cli/core/deployment_ledger.py +++ b/src/apm_cli/core/deployment_ledger.py @@ -193,21 +193,26 @@ def from_lockfile(lockfile: LockFile) -> DeploymentLedger: lockfile.local_deployed_files, lockfile.local_deployed_file_hashes, ) - for runtime, servers in getattr(lockfile, "mcp_target_servers", {}).items(): - for server in servers: - locator = DeploymentLocator( - kind=LocatorKind.URI, - target="mcp", - value=server, - runtime=runtime, - scope="project", - ) - records[locator.key] = DeploymentRecord( - locator=locator, - owners=(".",), - active_owner=".", - content_hash=None, - ) + service_targets = ( + ("mcp", getattr(lockfile, "mcp_target_servers", {})), + ("lsp", getattr(lockfile, "lsp_target_servers", {})), + ) + for service, target_servers in service_targets: + for runtime, servers in target_servers.items(): + for server in servers: + locator = DeploymentLocator( + kind=LocatorKind.URI, + target=service, + value=server, + runtime=runtime, + scope="project", + ) + records[locator.key] = DeploymentRecord( + locator=locator, + owners=(".",), + active_owner=".", + content_hash=None, + ) return DeploymentLedger(records=records) @staticmethod @@ -231,12 +236,14 @@ def apply_to_lockfile( } local_files: list[str] = [] local_hashes: dict[str, str] = {} - mcp_targets: dict[str, list[str]] = {} + service_targets: dict[str, dict[str, list[str]]] = {"mcp": {}, "lsp": {}} for record in ledger.records.values(): locator = record.locator - if locator.target == "mcp" and locator.runtime: - mcp_targets.setdefault(locator.runtime, []).append(locator.value) + if locator.target in service_targets and locator.runtime: + service_targets[locator.target].setdefault(locator.runtime, []).append( + locator.value + ) continue path = DeploymentLedgerCodec.legacy_value(locator) for owner in record.owners: @@ -258,7 +265,11 @@ def apply_to_lockfile( lockfile.local_deployed_file_hashes = dict(sorted(local_hashes.items())) lockfile.mcp_target_servers = { runtime: sorted(dict.fromkeys(servers)) - for runtime, servers in sorted(mcp_targets.items()) + for runtime, servers in sorted(service_targets["mcp"].items()) + } + lockfile.lsp_target_servers = { + runtime: sorted(dict.fromkeys(servers)) + for runtime, servers in sorted(service_targets["lsp"].items()) } @staticmethod @@ -359,6 +370,18 @@ def replace_mcp_target_servers( lockfile._mcp_target_servers_present = True DeploymentLedgerCodec.refresh_from_legacy(lockfile) + @staticmethod + def replace_lsp_target_servers( + lockfile: LockFile, + target_servers: dict[str, list[str]], + ) -> None: + """Update the LSP compatibility view and invalidate its projection.""" + lockfile.lsp_target_servers = { + runtime: list(servers) for runtime, servers in target_servers.items() + } + lockfile._lsp_target_servers_present = True + DeploymentLedgerCodec.refresh_from_legacy(lockfile) + @staticmethod def replace_context_local_files(context: Any, files: list[str]) -> None: """Route transitional install-context ownership mutation through one owner.""" @@ -429,6 +452,7 @@ def _rebuild_from_legacy( previous_by_locator = { ( record.locator.kind, + (record.locator.target if record.locator.kind == LocatorKind.URI else None), record.locator.value, record.locator.runtime, record.locator.scope, @@ -439,7 +463,13 @@ def _rebuild_from_legacy( for record in lockfile.deployment_ledger.records.values(): locator = record.locator previous = previous_by_locator.get( - (locator.kind, locator.value, locator.runtime, locator.scope) + ( + locator.kind, + locator.target if locator.kind == LocatorKind.URI else None, + locator.value, + locator.runtime, + locator.scope, + ) ) if previous is None: records[locator.key] = record diff --git a/src/apm_cli/core/null_logger.py b/src/apm_cli/core/null_logger.py index 16aa0aa41..8b0e4454b 100644 --- a/src/apm_cli/core/null_logger.py +++ b/src/apm_cli/core/null_logger.py @@ -5,7 +5,7 @@ method produces visible terminal output. Use this instead of ``logger=None`` checks inside ``MCPIntegrator`` -methods. It is NOT a drop-in replacement for the full +and ``LSPIntegrator`` methods. It is NOT a drop-in replacement for the full ``CommandLogger`` or ``InstallLogger`` interfaces used in CLI command functions. """ @@ -23,7 +23,7 @@ class NullCommandLogger: """Partial ``CommandLogger`` facade for ``MCPIntegrator`` contexts. Implements only the subset of ``CommandLogger`` needed by - ``MCPIntegrator``: ``start``, ``progress``, ``success``, + the integrators: ``start``, ``progress``, ``info``, ``success``, ``warning``, ``error``, ``verbose_detail``, ``tree_item``, and ``package_inline_warning``. @@ -51,6 +51,9 @@ def start(self, message: str, symbol: str = "running"): def progress(self, message: str, symbol: str = "info"): _rich_info(message, symbol=symbol) + def info(self, message: str, symbol: str = "info"): + _rich_info(message, symbol=symbol) + def mcp_lookup_heartbeat(self, count: int): """Mirror of ``CommandLogger.mcp_lookup_heartbeat`` (F4, #1116). diff --git a/src/apm_cli/deps/lockfile.py b/src/apm_cli/deps/lockfile.py index a714f29a4..cf63fb244 100644 --- a/src/apm_cli/deps/lockfile.py +++ b/src/apm_cli/deps/lockfile.py @@ -109,6 +109,7 @@ def _validate_lockfile_container(data: object) -> dict[str, Any]: "mcp_target_servers", "mcp_config_provenance", "lsp_configs", + "lsp_target_servers", "lsp_config_provenance", "local_deployed_file_hashes", ) @@ -125,13 +126,14 @@ def _validate_lockfile_container(data: object) -> dict[str, Any]: for index, dependency in enumerate(data.get("dependencies", [])): if not isinstance(dependency, dict): raise LockfileFormatError(f"Lockfile dependency at index {index} must be a mapping") - for target, servers in (data.get("mcp_target_servers") or {}).items(): - if not isinstance(target, str) or not target or not isinstance(servers, list): - raise LockfileFormatError( - "Lockfile mcp_target_servers values must be string-to-list mappings" - ) - if not all(isinstance(server, str) and bool(server) for server in servers): - raise LockfileFormatError("Lockfile mcp_target_servers entries must be strings") + for field_name in ("mcp_target_servers", "lsp_target_servers"): + for target, servers in (data.get(field_name) or {}).items(): + if not isinstance(target, str) or not target or not isinstance(servers, list): + raise LockfileFormatError( + f"Lockfile {field_name} values must be string-to-list mappings" + ) + if not all(isinstance(server, str) and bool(server) for server in servers): + raise LockfileFormatError(f"Lockfile {field_name} entries must be strings") for server, provenance in (data.get("mcp_config_provenance") or {}).items(): if not isinstance(server, str) or not ( (isinstance(provenance, str) and bool(provenance)) @@ -749,6 +751,7 @@ class LockFile: mcp_config_provenance: dict[str, str | list[str]] = field(default_factory=dict) lsp_servers: list[str] = field(default_factory=list) lsp_configs: dict[str, dict] = field(default_factory=dict) + lsp_target_servers: dict[str, list[str]] = field(default_factory=dict) lsp_config_provenance: dict[str, str] = field(default_factory=dict) local_deployed_files: list[str] = field(default_factory=list) local_deployed_file_hashes: dict[str, str] = field(default_factory=dict) @@ -757,6 +760,7 @@ class LockFile: ) _deployments_present: bool = field(default=False, repr=False, compare=False) _mcp_target_servers_present: bool = field(default=False, repr=False, compare=False) + _lsp_target_servers_present: bool = field(default=False, repr=False, compare=False) def add_dependency(self, dep: LockedDependency) -> None: """Add a dependency to the lock file. @@ -865,6 +869,11 @@ def to_yaml(self) -> str: data["lsp_servers"] = sorted(self.lsp_servers) if self.lsp_configs: data["lsp_configs"] = dict(sorted(self.lsp_configs.items())) + if self.lsp_target_servers: + data["lsp_target_servers"] = { + target: sorted(servers) + for target, servers in sorted(self.lsp_target_servers.items()) + } if self.lsp_config_provenance: data["lsp_config_provenance"] = dict(sorted(self.lsp_config_provenance.items())) if self.local_deployed_files: @@ -916,6 +925,11 @@ def _from_validated_data(cls, data: dict[str, Any]) -> LockFile: lock.mcp_config_provenance = dict(data.get("mcp_config_provenance") or {}) lock.lsp_servers = list(data.get("lsp_servers", [])) lock.lsp_configs = dict(data.get("lsp_configs") or {}) + lock.lsp_target_servers = { + target: list(servers) + for target, servers in (data.get("lsp_target_servers") or {}).items() + } + lock._lsp_target_servers_present = "lsp_target_servers" in data lock.lsp_config_provenance = dict(data.get("lsp_config_provenance") or {}) lock.local_deployed_files = list(data.get("local_deployed_files", [])) lock.local_deployed_file_hashes = dict(data.get("local_deployed_file_hashes") or {}) @@ -1147,6 +1161,8 @@ def is_semantically_equivalent(self, other: LockFile) -> bool: return False if self.lsp_configs != other.lsp_configs: return False + if self.lsp_target_servers != other.lsp_target_servers: + return False if self.lsp_config_provenance != other.lsp_config_provenance: return False # Issue #887: include hash dict in equivalence so post-install diff --git a/src/apm_cli/deps/path_anchoring.py b/src/apm_cli/deps/path_anchoring.py index e5266509a..0c923f1d3 100644 --- a/src/apm_cli/deps/path_anchoring.py +++ b/src/apm_cli/deps/path_anchoring.py @@ -60,6 +60,9 @@ def resolve_local_dep_dir( lock_dep: LockedDependency, lockfile: LockFile | None, project_root: Path, + *, + parent_index: dict[str, tuple[LockedDependency, ...]] | None = None, + resolved_cache: dict[str, Path] | None = None, ) -> Path: """Resolve the on-disk source directory for a locked LOCAL dependency. @@ -84,8 +87,47 @@ def resolve_local_dep_dir( f"not a resolvable local dependency: repo_url={lock_dep.repo_url!r} " f"source={lock_dep.source!r} local_path={lock_dep.local_path!r}" ) - anchor = _anchor_dir(lock_dep, lockfile, project_root.resolve(), seen=set()) - return _join(anchor, lock_dep.local_path) + if not lock_dep.resolved_by: + return _join(project_root.resolve(), lock_dep.local_path) + index = parent_index if parent_index is not None else build_local_parent_index(lockfile) + cache = resolved_cache if resolved_cache is not None else {} + return _resolve_dep_dir( + lock_dep, + lockfile, + project_root.resolve(), + parent_index=index, + resolved_cache=cache, + seen=set(), + ) + + +def build_local_parent_index( + lockfile: LockFile | None, +) -> dict[str, tuple[LockedDependency, ...]]: + """Index local dependencies by repo URL for repeated parent resolution.""" + grouped: dict[str, list[LockedDependency]] = {} + if lockfile is not None: + for dependency in lockfile.dependencies.values(): + if dependency.source == "local" and dependency.local_path: + grouped.setdefault(dependency.repo_url, []).append(dependency) + return {repo_url: tuple(dependencies) for repo_url, dependencies in grouped.items()} + + +def resolve_local_dep_dirs(lockfile: LockFile, project_root: Path) -> dict[str, Path]: + """Resolve every locked local dependency with one shared index and cache.""" + parent_index = build_local_parent_index(lockfile) + resolved_cache: dict[str, Path] = {} + return { + dependency.get_unique_key(): resolve_local_dep_dir( + dependency, + lockfile, + project_root, + parent_index=parent_index, + resolved_cache=resolved_cache, + ) + for dependency in lockfile.get_package_dependencies() + if dependency.source == "local" + } def _join(anchor: Path, local_path: str) -> Path: @@ -94,17 +136,25 @@ def _join(anchor: Path, local_path: str) -> Path: return raw.resolve() if raw.is_absolute() else (anchor / raw).resolve() -def _anchor_dir( +def _resolve_dep_dir( dep: LockedDependency, lockfile: LockFile | None, project_root: Path, + *, + parent_index: dict[str, tuple[LockedDependency, ...]], + resolved_cache: dict[str, Path], seen: set[str], ) -> Path: - """Return the directory on which *dep*'s ``local_path`` is anchored.""" + """Resolve one dependency, memoizing every parent in its ancestry.""" + key = dep.get_unique_key() + cached = resolved_cache.get(key) + if cached is not None: + return cached if not dep.resolved_by: - return project_root + resolved = _join(project_root, dep.local_path) + resolved_cache[key] = resolved + return resolved - key = dep.get_unique_key() if key in seen: raise LocalResolutionError( f"cycle in resolved_by chain at {dep.repo_url!r} " @@ -112,12 +162,27 @@ def _anchor_dir( ) seen.add(key) - parent = _find_parent(lockfile, dep) - parent_anchor = _anchor_dir(parent, lockfile, project_root, seen) - return _join(parent_anchor, parent.local_path) - - -def _find_parent(lockfile: LockFile | None, dep: LockedDependency) -> LockedDependency: + parent = _find_parent(lockfile, dep, parent_index=parent_index) + parent_dir = _resolve_dep_dir( + parent, + lockfile, + project_root, + parent_index=parent_index, + resolved_cache=resolved_cache, + seen=seen, + ) + resolved = _join(parent_dir, dep.local_path) + resolved_cache[key] = resolved + seen.remove(key) + return resolved + + +def _find_parent( + lockfile: LockFile | None, + dep: LockedDependency, + *, + parent_index: dict[str, tuple[LockedDependency, ...]], +) -> LockedDependency: """Find the unique local parent of *dep* by ``repo_url == resolved_by``. ``resolved_by`` carries the parent's ``repo_url`` (e.g. ``_local/foo``), @@ -132,11 +197,7 @@ def _find_parent(lockfile: LockFile | None, dep: LockedDependency) -> LockedDepe f"{dep.repo_url!r} declares resolved_by={dep.resolved_by!r} but no " "lockfile was supplied to resolve the parent" ) - matches = [ - other - for other in lockfile.dependencies.values() - if other.repo_url == dep.resolved_by and other.source == "local" and other.local_path - ] + matches = parent_index.get(dep.resolved_by, ()) if not matches: raise LocalResolutionError( f"resolved_by parent {dep.resolved_by!r} of {dep.repo_url!r} " diff --git a/src/apm_cli/deps/plugin_parser.py b/src/apm_cli/deps/plugin_parser.py index aa9429e4a..2662e94cd 100644 --- a/src/apm_cli/deps/plugin_parser.py +++ b/src/apm_cli/deps/plugin_parser.py @@ -599,7 +599,7 @@ def synthesize_apm_yml_from_plugin( substitute_plugin_root=substitute_plugin_root, ) if lsp_servers: - lsp_deps = _lsp_servers_to_apm_deps( + lsp_deps = lsp_servers_to_apm_deps( lsp_servers, plugin_path, warn_on_invalid=warn_on_invalid_servers, @@ -965,7 +965,7 @@ def _read_lsp_json(path: Path, logger: logging.Logger) -> dict[str, Any]: return dict(data) -def _lsp_servers_to_apm_deps( +def lsp_servers_to_apm_deps( servers: dict[str, Any], plugin_path: Path, *, @@ -1079,6 +1079,9 @@ def _lsp_servers_to_apm_deps( return deps +_lsp_servers_to_apm_deps = lsp_servers_to_apm_deps + + def _map_plugin_artifacts( plugin_path: Path, apm_dir: Path, manifest: dict[str, Any] | None = None ) -> None: diff --git a/src/apm_cli/deps/reachability.py b/src/apm_cli/deps/reachability.py index f0c98e86e..5e5927e70 100644 --- a/src/apm_cli/deps/reachability.py +++ b/src/apm_cli/deps/reachability.py @@ -42,7 +42,11 @@ from pathlib import Path from typing import TYPE_CHECKING -from apm_cli.deps.path_anchoring import LocalResolutionError, resolve_local_dep_dir +from apm_cli.deps.path_anchoring import ( + LocalResolutionError, + build_local_parent_index, + resolve_local_dep_dir, +) from apm_cli.models.apm_package import APMPackage if TYPE_CHECKING: @@ -122,12 +126,22 @@ def _build_local_dir_index( cannot rule out that some survivor reaches that entry. """ index: dict[Path, str] = {} + parent_index = build_local_parent_index(lockfile) + resolved_cache: dict[str, Path] = {} for key in candidate_orphans: dep = lockfile.get_dependency(key) if dep is None or dep.source != "local": continue try: - index[resolve_local_dep_dir(dep, lockfile, project_root)] = key + index[ + resolve_local_dep_dir( + dep, + lockfile, + project_root, + parent_index=parent_index, + resolved_cache=resolved_cache, + ) + ] = key except LocalResolutionError as exc: unverifiable.append((key, str(exc))) return index diff --git a/src/apm_cli/install/drift.py b/src/apm_cli/install/drift.py index c6bd857ad..c590c1c88 100644 --- a/src/apm_cli/install/drift.py +++ b/src/apm_cli/install/drift.py @@ -39,7 +39,13 @@ import click from apm_cli.core.command_logger import CommandLogger -from apm_cli.deps.path_anchoring import resolve_local_dep_dir +from apm_cli.deps.path_anchoring import build_local_parent_index, resolve_local_dep_dir +from apm_cli.install.drift_render import ( + _INLINE_DIFF_BYTE_CAP as _INLINE_DIFF_BYTE_CAP, +) +from apm_cli.install.drift_render import ( + _inline_diff_for, +) from apm_cli.install.drift_render import ( render_drift as render_drift, ) @@ -264,6 +270,8 @@ def _materialize_install_path( cache_only: bool, *, lockfile: LockFile | None = None, + parent_index: dict[str, tuple[LockedDependency, ...]] | None = None, + resolved_cache: dict[str, Path] | None = None, live_modules_dir: Path | None = None, downloader: Any | None = None, registry_resolver: Any | None = None, @@ -297,7 +305,13 @@ def _materialize_install_path( if lock_dep.source == "local": if not lock_dep.local_path: raise CacheMissError(f"local dep {lock_dep.repo_url!r} has no local_path in lockfile") - candidate = resolve_local_dep_dir(lock_dep, lockfile, project_root) + candidate = resolve_local_dep_dir( + lock_dep, + lockfile, + project_root, + parent_index=parent_index, + resolved_cache=resolved_cache, + ) if not candidate.exists(): raise CacheMissError( f"local source missing for {lock_dep.local_path!r}: expected {candidate}" @@ -624,6 +638,8 @@ def run_replay(config: ReplayConfig, logger: CheckLogger) -> Path: logger.replay_start() replayed_count = 0 + local_parent_index = build_local_parent_index(lock) + local_resolved_cache: dict[str, Path] = {} try: with _ReadOnlyProjectGuard(project_root, protected_subpaths): for lock_dep in lock.get_all_dependencies(): @@ -638,6 +654,8 @@ def run_replay(config: ReplayConfig, logger: CheckLogger) -> Path: apm_modules_dir, cache_only=config.cache_only, lockfile=lock, + parent_index=local_parent_index, + resolved_cache=local_resolved_cache, live_modules_dir=live_modules_dir, downloader=downloader, registry_resolver=registry_resolver, @@ -703,11 +721,7 @@ def run_replay(config: ReplayConfig, logger: CheckLogger) -> Path: return scratch_root -# --------------------------------------------------------------------------- # Diff engine -# --------------------------------------------------------------------------- - -_INLINE_DIFF_BYTE_CAP = 100 * 1024 # 100 KB def _governed_root_dirs(targets: list[TargetProfile]) -> set[str]: @@ -800,18 +814,6 @@ def _collect_hashed_files(lockfile: LockFile) -> set[str]: return set(DeploymentLedgerCodec.legacy_deployed_file_hash_paths(lockfile)) -def _inline_diff_for(scratch_path: Path, project_path: Path) -> str: - """Build an inline diff hint, capped to keep findings compact.""" - try: - s_size = scratch_path.stat().st_size - p_size = project_path.stat().st_size - except OSError: - return "" - if s_size > _INLINE_DIFF_BYTE_CAP or p_size > _INLINE_DIFF_BYTE_CAP: - return "(file too large for inline diff; use 'git diff --no-index' to compare)" - return "" - - def _canvas_deploy_prefixes(targets) -> set[str]: """Return ``root/subdir/`` prefixes for every target carrying a canvas mapping. diff --git a/src/apm_cli/install/drift_render.py b/src/apm_cli/install/drift_render.py index e67d93aab..32de39978 100644 --- a/src/apm_cli/install/drift_render.py +++ b/src/apm_cli/install/drift_render.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +from pathlib import Path from typing import TYPE_CHECKING from apm_cli.utils.console import STATUS_SYMBOLS @@ -10,6 +11,20 @@ if TYPE_CHECKING: from apm_cli.install.drift import DriftFinding +_INLINE_DIFF_BYTE_CAP = 100 * 1024 # 100 KB + + +def _inline_diff_for(scratch_path: Path, project_path: Path) -> str: + """Build an inline diff hint, capped to keep findings compact.""" + try: + scratch_size = scratch_path.stat().st_size + project_size = project_path.stat().st_size + except OSError: + return "" + if scratch_size > _INLINE_DIFF_BYTE_CAP or project_size > _INLINE_DIFF_BYTE_CAP: + return "(file too large for inline diff; use 'git diff --no-index' to compare)" + return "" + def render_drift_text(findings: list[DriftFinding], verbose: bool = False) -> str: """Render drift findings as grouped terminal text.""" diff --git a/src/apm_cli/install/local_bundle_handler.py b/src/apm_cli/install/local_bundle_handler.py index 00738d09d..4e01be079 100644 --- a/src/apm_cli/install/local_bundle_handler.py +++ b/src/apm_cli/install/local_bundle_handler.py @@ -59,7 +59,7 @@ def install_local_bundle( check_target_mismatch, verify_bundle_integrity, ) - from ..core.scope import InstallScope + from ..core.scope import InstallScope, get_source_root from ..deps.lockfile import LockFile, get_lockfile_path from ..install.services import ( enforce_agent_plugin_deployment_boundary, @@ -87,6 +87,7 @@ def install_local_bundle( scope = InstallScope.USER if global_ else InstallScope.PROJECT project_root = Path.home() if global_ else Path.cwd() + source_root = get_source_root(scope) logger.start(f"Installing local bundle from {bundle_arg}") @@ -149,29 +150,41 @@ def install_local_bundle( if bundle_mcp_declared and bundle_info.source_dir is not None else [] ) + bundle_lsp_deps = ( + _parse_bundle_lsp_servers(bundle_info.source_dir) + if bundle_info.source_dir is not None + else [] + ) + bundle_approval_key = None + if allow_executables is not None: + from ..security.executables import local_bundle_approval_key + + bundle_approval_key = local_bundle_approval_key( + bundle_info.package_id, + str(bundle_info.plugin_json.get("version") or ""), + bundle_info.source_dir, + bundle_info.lockfile, + ) bundle_mcp_deps = _filter_bundle_executables( bundle_mcp_deps, bundle_info=bundle_info, allow_executables=allow_executables, + approval_key=bundle_approval_key, exec_type="mcp", logger=logger, ) - bundle_lsp_deps = ( - _parse_bundle_lsp_servers(bundle_info.source_dir) - if bundle_info.source_dir is not None - else [] - ) bundle_lsp_deps = _filter_bundle_executables( bundle_lsp_deps, bundle_info=bundle_info, allow_executables=allow_executables, + approval_key=bundle_approval_key, exec_type="lsp", logger=logger, ) from ..policy.install_preflight import run_policy_preflight policy_fetch, _enforcement_active = run_policy_preflight( - project_root=project_root, + project_root=source_root, apm_deps=(), mcp_deps=bundle_mcp_deps, no_policy=no_policy, @@ -220,6 +233,17 @@ def install_local_bundle( if warning: logger.warning(warning) + if bundle_lsp_deps: + from apm_cli.install.errors import RequiredIntegrationError + from apm_cli.integration.lsp_integrator import LSPIntegrator + + target_names = [resolved.name for resolved in targets] + if not LSPIntegrator.supported_target_runtimes(target_names): + raise RequiredIntegrationError( + "Bundle lsp.json cannot be configured because the resolved target set has " + "no LSP-compatible runtime. Select --target claude or --target copilot." + ) + result = integrate_local_bundle( bundle_info, project_root, @@ -231,6 +255,7 @@ def install_local_bundle( scope=scope, alias=alias, allow_executables=allow_executables, + approval_key=bundle_approval_key, ) deployed = result.get("deployed_files", []) @@ -274,7 +299,7 @@ def install_local_bundle( deps=bundle_mcp_deps, owner=_bundle_owner_key(bundle_info), ) - if bundle_lsp_deps and bundle_info.source_dir is not None: + if bundle_info.source_dir is not None: _wire_bundle_lsp_servers( bundle_dir=bundle_info.source_dir, targets=targets, @@ -284,6 +309,7 @@ def install_local_bundle( logger=logger, deps=bundle_lsp_deps, owner=_bundle_owner_key(bundle_info), + force=force, ) # Persist into project lockfile -- never mutate apm.yml (per design). @@ -452,28 +478,58 @@ def _parse_legacy_bundle_mcp_servers( return out +def effective_bundle_allow_map( + project_root: Path, + *, + no_policy: bool, + logger: Any, +) -> dict[str, dict[str, bool]] | None: + """Resolve local-bundle trust through the canonical project owner.""" + from ..security.executables import effective_exec_map_for_project + + policy = None + if not no_policy: + from ..policy.discovery import discover_policy_with_chain + + policy = getattr( + discover_policy_with_chain(project_root, cache_only=True), + "policy", + None, + ) + return effective_exec_map_for_project( + project_root, + policy=policy, + logger=logger, + ) + + def _filter_bundle_executables( dependencies: list[Any], *, bundle_info, allow_executables: dict[str, dict[str, bool]] | None, + approval_key: str | None, exec_type: str, logger, ) -> list[Any]: - """Apply executable trust to a bundle identity, never to server names.""" + """Apply executable trust to the exact bundle artifact, never its claimed name.""" if not dependencies or allow_executables is None: return dependencies - from ..security.executables import build_approval_key, is_package_approved + from ..security.executables import is_package_approved - version = str(bundle_info.plugin_json.get("version") or "") - package_key = build_approval_key(bundle_info.package_id, version) - candidate_keys = (package_key, bundle_info.package_id) - if any(is_package_approved(allow_executables, key, exec_type) for key in candidate_keys): + if approval_key is None: + raise ValueError("Local bundle executable approval requires an artifact digest") + if is_package_approved(allow_executables, approval_key, exec_type): return dependencies + noun = "executable" if len(dependencies) == 1 else "executables" logger.warning( - f"Skipped {len(dependencies)} bundle {exec_type.upper()} executable(s) from " - f"{package_key}: approve the bundle identity under allowExecutables.{exec_type} " - "to enable them." + f"Skipped {len(dependencies)} bundle {exec_type.upper()} {noun} from " + f"{bundle_info.package_id}. To approve this exact local bundle, add:\n" + "executables:\n" + " allow:\n" + f' "{approval_key}":\n' + f" {exec_type}: true\n" + "Then rerun the install." ) return [] @@ -562,8 +618,6 @@ def _parse_bundle_lsp_servers( bundle_dir: Path, ): """Parse ``/lsp.json`` or ``/com.microsoft.apm/lsp.json`` into LSP deps.""" - from apm_cli.models.dependency.lsp import LSPDependency - lsp_path: Path | None = None for entry in bundle_dir.iterdir() if bundle_dir.is_dir() else []: if ( @@ -573,7 +627,7 @@ def _parse_bundle_lsp_servers( ): lsp_path = entry break - if entry.is_dir() and entry.name == COM_MICROSOFT_APM_NAMESPACE: + if entry.is_dir() and not entry.is_symlink() and entry.name == COM_MICROSOFT_APM_NAMESPACE: candidate = entry / "lsp.json" if candidate.is_file() and not candidate.is_symlink(): lsp_path = candidate @@ -593,17 +647,13 @@ def _parse_bundle_lsp_servers( if not isinstance(servers, dict): return [] - out: list[LSPDependency] = [] - for name, cfg in servers.items(): - if not isinstance(name, str) or not isinstance(cfg, dict): - continue - spec = dict(cfg) - spec["name"] = name - try: - out.append(LSPDependency.from_dict(spec)) - except (ValueError, TypeError): - continue - return out + from apm_cli.deps.plugin_parser import lsp_servers_to_apm_deps + from apm_cli.models.dependency.lsp import LSPDependency + + return [ + LSPDependency.from_dict(spec) + for spec in lsp_servers_to_apm_deps(servers, lsp_path, warn_on_invalid=False) + ] def _wire_bundle_lsp_servers( @@ -616,12 +666,15 @@ def _wire_bundle_lsp_servers( logger, deps, owner: str, + force: bool = False, ) -> int: """Wire bundle LSP servers through the canonical owned lifecycle.""" from apm_cli.deps.lockfile import get_lockfile_path from apm_cli.install.lsp.integration import run_owned_lsp_integration + from apm_cli.integration.lsp_integrator import LSPIntegrator target_names = [t.name for t in targets] + lsp_target_names = LSPIntegrator.supported_target_runtimes(target_names) count = run_owned_lsp_integration( dependencies=deps, owner=owner, @@ -629,16 +682,17 @@ def _wire_bundle_lsp_servers( project_root=project_root, user_scope=user_scope, logger=logger, - target_runtimes=target_names, + target_runtimes=lsp_target_names, fail_on_write_error=True, + force=force, ) if count: logger.success( - f"Wired {count} LSP server(s) from bundle lsp.json (target(s): {', '.join(target_names)})" + f"Wired {count} LSP server(s) from bundle lsp.json (target(s): {', '.join(lsp_target_names)})" ) elif deps: logger.info( - f"Bundle lsp.json declared {len(deps)} server(s); no new LSP config changes for target(s): {', '.join(target_names)}" + f"Bundle lsp.json declared {len(deps)} server(s); no new LSP config changes for target(s): {', '.join(lsp_target_names)}" ) return count diff --git a/src/apm_cli/install/lsp/integration.py b/src/apm_cli/install/lsp/integration.py index 94fdf1454..34efc0ee0 100644 --- a/src/apm_cli/install/lsp/integration.py +++ b/src/apm_cli/install/lsp/integration.py @@ -13,6 +13,73 @@ from apm_cli.models.apm_package import APMPackage from apm_cli.models.dependency.lsp import LSPDependency +_PROJECT_LSP_OWNER = "project:." +_PACKAGE_LSP_OWNER_PREFIX = "package:" +_BUNDLE_LSP_OWNER_PREFIX = "bundle:" +_RESERVED_LSP_OWNER_PREFIXES = ( + _PROJECT_LSP_OWNER.split(".", 1)[0], + _PACKAGE_LSP_OWNER_PREFIX, + _BUNDLE_LSP_OWNER_PREFIX, +) + + +def _target_server_sets(lockfile: "LockFile") -> dict[str, set[str]]: + """Return a mutable target ownership view from one lockfile.""" + target_servers = getattr(lockfile, "lsp_target_servers", {}) + if not isinstance(target_servers, dict): + return {} + return {runtime: set(server_names) for runtime, server_names in target_servers.items()} + + +def _dependency_provenance(dependencies: list["LSPDependency"]) -> dict[str, str]: + """Map every regular install declaration to its stable owner token.""" + return { + dependency.name: ( + f"{_PACKAGE_LSP_OWNER_PREFIX}{dependency.resolved_by}" + if dependency.resolved_by + else _PROJECT_LSP_OWNER + ) + for dependency in dependencies + } + + +def _is_regular_owner(owner: str) -> bool: + """Return whether an LSP owner belongs to the replayable install graph.""" + return owner == _PROJECT_LSP_OWNER or owner.startswith(_PACKAGE_LSP_OWNER_PREFIX) + + +def _bundle_owner_aliases(owner: str) -> frozenset[str]: + """Return canonical and safe legacy provenance tokens for one bundle.""" + canonical = f"{_BUNDLE_LSP_OWNER_PREFIX}{owner}" + if owner.startswith(_RESERVED_LSP_OWNER_PREFIXES): + return frozenset({canonical}) + return frozenset({canonical, owner}) + + +def _clean_target_differences( + *, + old_targets: dict[str, set[str]], + new_targets: dict[str, set[str]], + project_root: Path, + user_scope: bool, + logger, + fail_on_write_error: bool, +) -> None: + """Remove only target-scoped LSP entries whose recorded ownership was dropped.""" + from apm_cli.integration.lsp_integrator import LSPIntegrator + + for runtime, old_names in old_targets.items(): + stale = old_names - new_targets.get(runtime, set()) + if stale: + LSPIntegrator.remove_stale( + stale, + project_root=project_root, + user_scope=user_scope, + logger=logger, + target_runtimes=[runtime], + fail_on_write_error=fail_on_write_error, + ) + def run_owned_lsp_integration( *, @@ -24,16 +91,19 @@ def run_owned_lsp_integration( target_runtimes: list[str], logger, fail_on_write_error: bool = True, + force: bool = False, ) -> int: """Reconcile one bundle owner's LSP servers and persist ownership.""" from apm_cli.deps.lockfile import LockFile from apm_cli.integration.lsp_integrator import LSPIntegrator lockfile = LockFile.read(lock_path) or LockFile() + owner_token = f"{_BUNDLE_LSP_OWNER_PREFIX}{owner}" + owner_aliases = _bundle_owner_aliases(owner) old_owned = { name for name, recorded_owner in lockfile.lsp_config_provenance.items() - if recorded_owner == owner + if recorded_owner in owner_aliases } new_names = LSPIntegrator.get_server_names(dependencies) conflicts = { @@ -41,12 +111,36 @@ def run_owned_lsp_integration( for name in new_names if name in lockfile.lsp_servers and name not in old_owned - and lockfile.lsp_config_provenance.get(name) != owner + and lockfile.lsp_config_provenance.get(name) not in owner_aliases } if conflicts: + conflict_details = ", ".join( + f"{name} (owned by {lockfile.lsp_config_provenance.get(name, 'legacy lock state')})" + for name in sorted(conflicts) + ) raise ValueError( - "Bundle LSP server name conflicts with another owner: " + ", ".join(sorted(conflicts)) + "Bundle LSP server name conflicts with another owner: " + f"{conflict_details}. Rename the declaration or remove and reinstall " + "the owning bundle; --force does not transfer ownership." + ) + + if not dependencies and not old_owned: + return 0 + + old_targets = _target_server_sets(lockfile) + new_targets = {runtime: set(names) for runtime, names in old_targets.items()} + for names in new_targets.values(): + names.difference_update(old_owned) + supported_target_runtimes = LSPIntegrator.supported_target_runtimes(target_runtimes) + if dependencies and not supported_target_runtimes: + from apm_cli.install.errors import RequiredIntegrationError + + raise RequiredIntegrationError( + "Bundle lsp.json cannot be configured because the resolved target set has " + "no LSP-compatible runtime. Select --target claude or --target copilot." ) + for runtime in supported_target_runtimes: + new_targets.setdefault(runtime, set()).update(new_names) count = 0 if dependencies: @@ -55,30 +149,99 @@ def run_owned_lsp_integration( project_root=project_root, user_scope=user_scope, logger=logger, - target_runtimes=target_runtimes, - fail_on_write_error=fail_on_write_error, - ) - stale = old_owned - new_names - if stale: - LSPIntegrator.remove_stale( - stale, - project_root=project_root, - user_scope=user_scope, - logger=logger, - target_runtimes=target_runtimes, + target_runtimes=supported_target_runtimes, fail_on_write_error=fail_on_write_error, + managed_target_servers=old_targets, + force=force, ) + _clean_target_differences( + old_targets=old_targets, + new_targets=new_targets, + project_root=project_root, + user_scope=user_scope, + logger=logger, + fail_on_write_error=fail_on_write_error, + ) for name in old_owned: lockfile.lsp_configs.pop(name, None) lockfile.lsp_config_provenance.pop(name, None) lockfile.lsp_servers = sorted((set(lockfile.lsp_servers) - old_owned) | new_names) lockfile.lsp_configs.update(LSPIntegrator.get_server_configs(dependencies)) - lockfile.lsp_config_provenance.update(dict.fromkeys(new_names, owner)) + lockfile.lsp_config_provenance.update(dict.fromkeys(new_names, owner_token)) + from apm_cli.core.deployment_ledger import DeploymentLedgerCodec + + DeploymentLedgerCodec.replace_lsp_target_servers( + lockfile, + { + runtime: sorted(server_names) + for runtime, server_names in new_targets.items() + if server_names + }, + ) lockfile.write(lock_path) return count +def reconcile_lsp_after_uninstall( + *, + apm_package: "APMPackage", + lockfile: "LockFile | None", + lock_path: Path, + modules_dir: Path, + project_root: Path, + user_scope: bool, + logger, +) -> bool: + """Recompute trusted LSP state from every surviving declaration.""" + if lockfile is None or not ( + lockfile.lsp_servers or lockfile.lsp_target_servers or lockfile.lsp_config_provenance + ): + return False + if apm_package is None: + raise ValueError("Cannot reconcile existing LSP state without a valid project manifest") + before = ( + list(lockfile.lsp_servers), + dict(lockfile.lsp_configs), + dict(lockfile.lsp_target_servers), + dict(lockfile.lsp_config_provenance), + ) + from apm_cli.core.scope import InstallScope + from apm_cli.core.target_detection import resolve_package_target_decision + from apm_cli.models.apm_package import canonical_package_target_config + + scope = InstallScope.USER if user_scope else InstallScope.PROJECT + target_decision = resolve_package_target_decision( + project_root, + package=apm_package, + explicit_target=None, + user_scope=user_scope, + ) + apm_config = {"scripts": apm_package.scripts or {}} + apm_config.update(canonical_package_target_config(apm_package)) + run_lsp_integration( + apm_package=apm_package, + apm_modules_path=modules_dir, + lock_path=lock_path, + existing_lock=lockfile, + project_root=project_root, + user_scope=user_scope, + should_install=True, + logger=logger, + target_context=(apm_config, target_decision.value, scope), + target_decision=target_decision, + fail_on_write_error=True, + persist=False, + ) + after = ( + list(lockfile.lsp_servers), + dict(lockfile.lsp_configs), + dict(lockfile.lsp_target_servers), + dict(lockfile.lsp_config_provenance), + ) + return before != after + + def run_lsp_integration( # noqa: PLR0913 *, apm_package: "APMPackage", @@ -98,6 +261,11 @@ def run_lsp_integration( # noqa: PLR0913 target_context: tuple[dict | None, str | list[str] | None, object] | None = None, target_decision: "EffectiveTargetDecision | None" = None, fail_on_write_error: bool = False, + effective_allow_executables: dict[str, dict[str, bool]] | None = None, + effective_allow_resolved: bool = False, + force: bool = False, + no_policy: bool = False, + persist: bool = True, ) -> int: """Run LSP server integration after APM package installation. @@ -140,11 +308,23 @@ def run_lsp_integration( # noqa: PLR0913 # Capture old LSP servers from lockfile old_lsp_servers: builtins.set = builtins.set() old_lsp_configs: builtins.dict = {} + old_lsp_provenance: dict[str, str] = {} + old_lsp_targets: dict[str, set[str]] = {} + old_lsp_targets_present = False if existing_lock: old_lsp_servers = builtins.set(existing_lock.lsp_servers) old_lsp_configs = builtins.dict(existing_lock.lsp_configs) + raw_provenance = getattr(existing_lock, "lsp_config_provenance", {}) + old_lsp_provenance = dict(raw_provenance) if isinstance(raw_provenance, dict) else {} + old_lsp_targets = _target_server_sets(existing_lock) + old_lsp_targets_present = ( + getattr(existing_lock, "_lsp_target_servers_present", False) is True + ) + + from apm_cli.security.executables import filter_lsp_by_allow_executables - # Collect transitive LSP deps from installed packages + # Filter transitive declarations before first-wins deduplication so an + # untrusted package cannot shadow an approved package's same-name server. if should_install and apm_modules_path.exists(): transitive_lsp = LSPIntegrator.collect_transitive( apm_modules_path, @@ -153,17 +333,43 @@ def run_lsp_integration( # noqa: PLR0913 ) if transitive_lsp: logger.verbose_detail(f"Collected {len(transitive_lsp)} transitive LSP dependency(ies)") - lsp_deps = LSPIntegrator.deduplicate(lsp_deps + transitive_lsp) + if not effective_allow_resolved: + from apm_cli.security.executables import effective_exec_map_for_project - from apm_cli.security.executables import filter_lsp_by_allow_executables + policy = None + if not no_policy: + from apm_cli.policy.discovery import discover_policy_with_chain - package_allow = getattr(apm_package, "allow_executables", None) - if not isinstance(package_allow, dict): - package_allow = None - lsp_deps = filter_lsp_by_allow_executables(lsp_deps, package_allow, logger) + policy = getattr( + discover_policy_with_chain(project_root), + "policy", + None, + ) + effective_allow_executables = effective_exec_map_for_project( + project_root, + policy=policy, + fallback_allow_executables=getattr( + apm_package, + "allow_executables", + None, + ), + logger=logger, + ) + effective_allow_resolved = True + transitive_lsp = filter_lsp_by_allow_executables( + transitive_lsp, + effective_allow_executables, + logger, + ) + lsp_deps = LSPIntegrator.deduplicate(lsp_deps + transitive_lsp) + + if should_install and not ( + lsp_deps or old_lsp_servers or old_lsp_provenance or old_lsp_targets + ): + logger.verbose_detail("No LSP dependencies found in apm.yml") + return 0 lsp_count = 0 - new_lsp_servers: builtins.set = builtins.set() if target_context is not None: apm_config, explicit_target, scope = target_context @@ -182,71 +388,98 @@ def run_lsp_integration( # noqa: PLR0913 logger=logger, ) - if should_install and lsp_deps: - if not target_runtimes and fail_on_write_error: - from apm_cli.install.errors import RequiredIntegrationError - - raise RequiredIntegrationError( - "LSP dependencies are declared, but no effective target supports " - "LSP configuration. Choose --target claude or --target copilot, then retry." + if should_install: + bundle_names = { + name for name, owner in old_lsp_provenance.items() if not _is_regular_owner(owner) + } + old_regular_names = { + name for name, owner in old_lsp_provenance.items() if _is_regular_owner(owner) + } + if old_lsp_targets_present: + old_regular_names.update(set().union(*old_lsp_targets.values(), set()) - bundle_names) + new_regular_names = LSPIntegrator.get_server_names(lsp_deps) if lsp_deps else builtins.set() + conflicts = new_regular_names & bundle_names + if conflicts: + conflict_details = ", ".join( + f"{name} (owned by {old_lsp_provenance[name]})" for name in sorted(conflicts) + ) + raise ValueError( + "Manifest LSP server name conflicts with an installed bundle owner: " + f"{conflict_details}. Rename the declaration or remove and reinstall " + "the owning bundle; --force does not transfer ownership." ) - lsp_count = LSPIntegrator.install( - lsp_deps, - project_root=project_root, - user_scope=user_scope, - logger=logger, - diagnostics=diagnostics, - target_runtimes=target_runtimes, - fail_on_write_error=fail_on_write_error, - ) - new_lsp_servers = LSPIntegrator.get_server_names(lsp_deps) - new_lsp_configs = LSPIntegrator.get_server_configs(lsp_deps) - # Remove stale LSP servers - stale_lsp = old_lsp_servers - new_lsp_servers - if stale_lsp: - LSPIntegrator.remove_stale( - stale_lsp, + if lsp_deps: + if not target_runtimes and fail_on_write_error: + from apm_cli.install.errors import RequiredIntegrationError + + raise RequiredIntegrationError( + "LSP dependencies are declared, but no effective target supports " + "LSP configuration. Choose --target claude or --target copilot, then retry." + ) + lsp_count = LSPIntegrator.install( + lsp_deps, project_root=project_root, user_scope=user_scope, logger=logger, + diagnostics=diagnostics, target_runtimes=target_runtimes, fail_on_write_error=fail_on_write_error, + managed_target_servers=old_lsp_targets, + force=force, ) - # Persist LSP servers in lockfile - LSPIntegrator.update_lockfile( - new_lsp_servers, - lock_path, - lsp_configs=new_lsp_configs, - fail_on_write_error=fail_on_write_error, - ) - - elif should_install and not lsp_deps: - # No LSP deps -- remove any old APM-managed servers - if old_lsp_servers: - LSPIntegrator.remove_stale( - old_lsp_servers, + new_targets = { + runtime_name: set(server_names) + for runtime_name, server_names in old_lsp_targets.items() + } + for server_names in new_targets.values(): + server_names.difference_update(old_regular_names) + for target_runtime in target_runtimes or []: + new_targets.setdefault(target_runtime, set()).update(new_regular_names) + if old_lsp_targets_present: + _clean_target_differences( + old_targets=old_lsp_targets, + new_targets=new_targets, project_root=project_root, user_scope=user_scope, logger=logger, - target_runtimes=target_runtimes, - fail_on_write_error=fail_on_write_error, - ) - LSPIntegrator.update_lockfile( - builtins.set(), - lock_path, - lsp_configs={}, fail_on_write_error=fail_on_write_error, ) - logger.verbose_detail("No LSP dependencies found in apm.yml") - elif not should_install and old_lsp_servers: - # --only=apm: restore old LSP servers + new_regular_configs = LSPIntegrator.get_server_configs(lsp_deps) + new_configs = { + name: config for name, config in old_lsp_configs.items() if name in bundle_names + } + new_configs.update(new_regular_configs) + new_provenance = { + name: owner for name, owner in old_lsp_provenance.items() if name in bundle_names + } + new_provenance.update(_dependency_provenance(lsp_deps)) + all_names = bundle_names | new_regular_names + LSPIntegrator.update_lockfile( + all_names, + lock_path, + lsp_configs=new_configs, + lsp_target_servers=new_targets, + lsp_config_provenance=new_provenance, + lockfile_state=existing_lock if not persist else None, + persist=persist, + fail_on_write_error=fail_on_write_error, + ) + if not lsp_deps: + logger.verbose_detail("No LSP dependencies found in apm.yml") + + elif old_lsp_servers: + # Selective APM or MCP installs preserve every LSP ownership view. LSPIntegrator.update_lockfile( old_lsp_servers, lock_path, lsp_configs=old_lsp_configs, + lsp_target_servers=old_lsp_targets, + lsp_config_provenance=old_lsp_provenance, + lockfile_state=existing_lock if not persist else None, + persist=persist, fail_on_write_error=fail_on_write_error, ) diff --git a/src/apm_cli/install/outcome.py b/src/apm_cli/install/outcome.py index 53004dc06..1f530f79e 100644 --- a/src/apm_cli/install/outcome.py +++ b/src/apm_cli/install/outcome.py @@ -67,6 +67,8 @@ def result_from_install_context(ctx: InstallContext) -> InstallResult: ctx.diagnostics, package_types=dict(ctx.package_types), target_decision=getattr(ctx, "target_decision", None), + exec_allow_map=getattr(ctx, "exec_allow_map", None), + exec_allow_resolved=getattr(ctx, "exec_trust_ctx", None) is not None, ), force=bool(getattr(ctx, "force", False)), ) diff --git a/src/apm_cli/install/phases/lockfile.py b/src/apm_cli/install/phases/lockfile.py index 288a16aaf..d0b6a0d37 100644 --- a/src/apm_cli/install/phases/lockfile.py +++ b/src/apm_cli/install/phases/lockfile.py @@ -471,6 +471,17 @@ def _preserve_existing_lsp_state(self, lockfile: LockFile) -> None: if self.ctx.existing_lockfile: lockfile.lsp_servers = list(self.ctx.existing_lockfile.lsp_servers) lockfile.lsp_configs = copy.deepcopy(self.ctx.existing_lockfile.lsp_configs) + lockfile.lsp_config_provenance = copy.deepcopy( + self.ctx.existing_lockfile.lsp_config_provenance + ) + target_servers = self.ctx.existing_lockfile.lsp_target_servers + if target_servers: + from apm_cli.core.deployment_ledger import DeploymentLedgerCodec + + DeploymentLedgerCodec.replace_lsp_target_servers( + lockfile, + copy.deepcopy(target_servers), + ) if self.ctx.logger: self.ctx.logger.verbose_detail( "LSP state unchanged -- carrying forward " diff --git a/src/apm_cli/install/service_integration.py b/src/apm_cli/install/service_integration.py index 26d07ff2b..0af3e9cd7 100644 --- a/src/apm_cli/install/service_integration.py +++ b/src/apm_cli/install/service_integration.py @@ -27,6 +27,9 @@ class ServiceCommandContext(Protocol): trust_transitive_mcp: bool no_policy: bool verbose: bool + force: bool + exec_allow_map: dict[str, dict[str, bool]] | None + exec_allow_resolved: bool @dataclass(frozen=True) @@ -60,7 +63,8 @@ def run_service_integrations( from apm_cli.install.lsp import run_lsp_integration from apm_cli.install.mcp import run_mcp_integration - should_install = ctx.install_mode != InstallMode.APM + should_install_mcp = ctx.install_mode != InstallMode.APM + should_install_lsp = ctx.install_mode is InstallMode.ALL lsp_deps = apm_package.get_lsp_dependencies() if not isinstance(lsp_deps, list): ctx.logger.verbose_detail("LSP dependencies were not a list; defaulting to empty") @@ -68,7 +72,7 @@ def run_service_integrations( old_lsp_servers = set(existing_lock.lsp_servers) if existing_lock else set() if ( target_decision is None - and should_install + and (should_install_mcp or should_install_lsp) and (mcp_deps or lsp_deps or old_mcp_servers or old_lsp_servers) ): target_decision = resolve_package_target_decision( @@ -91,7 +95,7 @@ def run_service_integrations( old_mcp_target_servers_present=old_mcp_target_servers_present, project_root=ctx.project_root, user_scope=ctx.scope is InstallScope.USER, - should_install=should_install, + should_install=should_install_mcp, logger=ctx.logger, diagnostics=diagnostics, runtime=ctx.runtime, @@ -110,7 +114,7 @@ def run_service_integrations( existing_lock=existing_lock, project_root=ctx.project_root, user_scope=ctx.scope is InstallScope.USER, - should_install=should_install, + should_install=should_install_lsp, logger=ctx.logger, diagnostics=diagnostics, runtime=ctx.runtime, @@ -122,5 +126,9 @@ def run_service_integrations( ), target_decision=target_decision, fail_on_write_error=True, + effective_allow_executables=ctx.exec_allow_map, + effective_allow_resolved=ctx.exec_allow_resolved, + force=ctx.force, + no_policy=ctx.no_policy, ) return ServiceIntegrationResult(mcp_count, lsp_count, target_decision) diff --git a/src/apm_cli/install/services.py b/src/apm_cli/install/services.py index 84114d6d6..8cb07fe39 100644 --- a/src/apm_cli/install/services.py +++ b/src/apm_cli/install/services.py @@ -794,6 +794,7 @@ def integrate_local_bundle( scope: InstallScope | None = None, alias: str | None = None, allow_executables: builtins.dict[str, builtins.dict[str, bool]] | None = None, + approval_key: str | None = None, ) -> dict: """Integrate a detected local bundle into project / user scope. @@ -823,11 +824,8 @@ def integrate_local_bundle( logger: Install-flow logger. scope: ``InstallScope`` (project vs user) for downstream consumers. alias: Slug override from ``--as``. - allow_executables: The ``allowExecutables`` block from the consuming - project's ``apm.yml``. When ``None`` (no enforcement), all - executable primitives including canvas are allowed. When - provided, canvas extensions from the bundle are only deployed if - the bundle slug is approved for the ``canvas`` exec type. + allow_executables: Effective executable approvals, or ``None`` when disabled. + approval_key: Exact local-bundle content identity for executable approval. Returns: Dict with keys ``deployed_files`` (list[str]), @@ -924,8 +922,9 @@ def _normalized_bundle_text(path: Path) -> str | None: if _canvas_enabled: from ..security.executables import EXEC_TYPE_CANVAS, is_package_approved - _canvas_approved_bundle = allow_executables is None or is_package_approved( - allow_executables, slug, EXEC_TYPE_CANVAS + _canvas_approved_bundle = allow_executables is None or ( + approval_key is not None + and is_package_approved(allow_executables, approval_key, EXEC_TYPE_CANVAS) ) else: _canvas_approved_bundle = False @@ -941,8 +940,13 @@ def _normalized_bundle_text(path: Path) -> str | None: _msg = ( f"Blocked {len(_blocked)} canvas extension file(s) from bundle " f"'{slug}': canvas extensions are executable extension.mjs code " - f"and are not approved in allowExecutables. " - f"Run 'apm approve {slug}' to approve them." + "and are not approved for this exact bundle content. " + "Add this to apm.yml:\n" + "executables:\n" + " allow:\n" + f' "{approval_key}":\n' + " canvas: true\n" + "Then rerun the install." ) if diagnostics is not None: diagnostics.warn(message=_msg, package=str(slug)) diff --git a/src/apm_cli/install/template.py b/src/apm_cli/install/template.py index 49f78db11..508e64fb4 100644 --- a/src/apm_cli/install/template.py +++ b/src/apm_cli/install/template.py @@ -52,38 +52,22 @@ def _effective_allow(ctx) -> dict | None: executable deploys). """ from apm_cli.security.executables import ( - build_exec_trust_context, + exec_trust_context_for_project, materialize_exec_map, ) - from apm_cli.utils.yaml_io import load_yaml if getattr(ctx, "exec_trust_ctx", None) is not None: return getattr(ctx, "exec_allow_map", None) - project_data: dict | None = None - manifest = getattr(ctx, "project_root", None) - if manifest is not None: - manifest_path = manifest / "apm.yml" - if manifest_path.is_file(): - data = load_yaml(manifest_path) - if isinstance(data, dict): - project_data = data - if data.get("allowExecutables") is not None: - from apm_cli.security.executables import ( - warn_allow_executables_alias_once, - ) - - warn_allow_executables_alias_once(getattr(ctx, "logger", None)) - - # Fall back to the in-memory gate signal when apm.yml is unreadable so a - # project that opted in via allowExecutables still gates. - if project_data is None: - project_val = getattr(getattr(ctx, "apm_package", None), "allow_executables", None) - if isinstance(project_val, dict): - project_data = {"allowExecutables": project_val} - policy = getattr(getattr(ctx, "policy_fetch", None), "policy", None) - trust_ctx = build_exec_trust_context(policy=policy, project_data=project_data) + project_root = getattr(ctx, "project_root", None) + project_allow = getattr(getattr(ctx, "apm_package", None), "allow_executables", None) + trust_ctx = exec_trust_context_for_project( + project_root, + policy=policy, + fallback_allow_executables=project_allow, + logger=getattr(ctx, "logger", None), + ) allow_map = materialize_exec_map(trust_ctx) # Cache the resolved context and allow map once per install so each # dependency uses the same precedence ladder without re-reading policy files. diff --git a/src/apm_cli/integration/_shared.py b/src/apm_cli/integration/_shared.py index 24c1e38e0..19e2b850d 100644 --- a/src/apm_cli/integration/_shared.py +++ b/src/apm_cli/integration/_shared.py @@ -37,35 +37,56 @@ def deduplicate_deps(deps: list) -> list: return result -def resolve_locked_apm_yml_paths( +def resolve_locked_apm_yml_sources( apm_modules_dir: Path, lock_path: Path | None, -) -> tuple[list[Path] | None, builtins.set]: - """Resolve apm.yml paths from the lockfile. +) -> tuple[list[tuple[Path, object]] | None, builtins.set]: + """Resolve package manifest paths and dependency records from the lockfile. - Returns ``(locked_paths_or_None, direct_paths_set)``. - When *locked_paths* is ``None`` the caller should fall back to rglob. + Returns ``(locked_sources_or_None, direct_paths_set)``. Each source pairs an + ``apm.yml`` path with its locked dependency so consumers can retain exact + identity and provenance. When *locked_sources* is ``None`` the caller should + fall back to rglob. """ - locked_paths: builtins.set | None = None + locked_sources: dict[Path, object] | None = None direct_paths: builtins.set = builtins.set() if lock_path and lock_path.exists(): lockfile = LockFile.read(lock_path) if lockfile is not None: - locked_paths = builtins.set() + locked_sources = {} for dep in lockfile.get_package_dependencies(): if dep.repo_url: - yml = ( - apm_modules_dir / dep.repo_url / dep.virtual_path / "apm.yml" - if dep.virtual_path - else apm_modules_dir / dep.repo_url / "apm.yml" - ) - locked_paths.add(yml.resolve()) + package_root = dep.to_dependency_ref().get_install_path(apm_modules_dir) + yml = package_root / "apm.yml" + if yml.is_symlink(): + raise ValueError(f"Locked package manifest must not be a symlink: {yml}") + resolved_yml = yml.resolve() + try: + resolved_yml.relative_to(package_root.resolve()) + except ValueError as exc: + raise ValueError( + f"Locked package manifest escapes its package root: {yml}" + ) from exc + locked_sources[resolved_yml] = dep if dep.depth == 1: - direct_paths.add(yml.resolve()) + direct_paths.add(resolved_yml) - if locked_paths is not None: - resolved = [path for path in sorted(locked_paths) if path.exists()] + if locked_sources is not None: + resolved = [ + (path, locked_sources[path]) for path in sorted(locked_sources) if path.exists() + ] return resolved, direct_paths return None, direct_paths + + +def resolve_locked_apm_yml_paths( + apm_modules_dir: Path, + lock_path: Path | None, +) -> tuple[list[Path] | None, builtins.set]: + """Resolve manifest paths while preserving the legacy path-only API.""" + sources, direct_paths = resolve_locked_apm_yml_sources(apm_modules_dir, lock_path) + if sources is None: + return None, direct_paths + return [path for path, _dependency in sources], direct_paths diff --git a/src/apm_cli/integration/base_integrator.py b/src/apm_cli/integration/base_integrator.py index 52396f289..af7a27f77 100644 --- a/src/apm_cli/integration/base_integrator.py +++ b/src/apm_cli/integration/base_integrator.py @@ -9,10 +9,19 @@ from apm_cli.compilation.link_resolver import UnifiedLinkResolver from apm_cli.core.deployment_state import MaterializationResult -from apm_cli.primitives.discovery import discover_primitives from apm_cli.utils.atomic_io import normalize_crlf_to_lf from apm_cli.utils.console import _rich_warning -from apm_cli.utils.path_security import PathTraversalError, ensure_path_within +from apm_cli.utils.path_security import ( + PathTraversalError, + ensure_path_within, + has_symlink_component, +) + + +def discover_primitives(*args: Any, **kwargs: Any) -> Any: + from apm_cli.primitives.discovery import discover_primitives as _discover_primitives + + return _discover_primitives(*args, **kwargs) def _managed_absolute_target_root(candidate: Path, targets: Any) -> Path | None: @@ -553,6 +562,28 @@ def validate_deploy_path( return False return True + @staticmethod + def resolve_deploy_path( + rel_path: str, + project_root: Path, + allowed_prefixes: tuple | None = None, + targets=None, + ) -> Path: + """Return a safe project deployment path without following symlinks.""" + target = project_root / rel_path + if has_symlink_component(project_root, target): + raise PathTraversalError( + f"Refusing deployment through a symlinked path component: {rel_path}" + ) + if not BaseIntegrator.validate_deploy_path( + rel_path, + project_root, + allowed_prefixes=allowed_prefixes, + targets=targets, + ): + raise PathTraversalError(f"Refusing unsafe deployment path: {rel_path}") + return ensure_path_within(target, project_root) + # Backward-compat aliases mapping raw ``{prim}_{target}`` keys to # the bucket names that existing callers expect. Shared between # ``partition_managed_files`` and ``partition_bucket_key`` so the diff --git a/src/apm_cli/integration/lsp_integrator.py b/src/apm_cli/integration/lsp_integrator.py index 2396e1989..5eafbd037 100644 --- a/src/apm_cli/integration/lsp_integrator.py +++ b/src/apm_cli/integration/lsp_integrator.py @@ -1,8 +1,7 @@ -"""Standalone LSP lifecycle orchestrator. +"""Runtime target adapter for LSP configuration. -Owns LSP dependency resolution, installation, stale cleanup, and lockfile -persistence logic. LSP config is written through runtime targets so vendor -specific path and field differences stay isolated behind a neutral interface. +Owns target paths, shapes, file writes, and cleanup mechanics. The install LSP +pipeline owns collection, trust filtering, and lifecycle reconciliation. """ from __future__ import annotations @@ -10,13 +9,17 @@ import builtins import json import logging -from dataclasses import dataclass +from dataclasses import dataclass, replace from pathlib import Path from typing import TYPE_CHECKING from apm_cli.core.null_logger import NullCommandLogger from apm_cli.deps.lockfile import LockFile, get_lockfile_path -from apm_cli.integration._shared import deduplicate_deps, resolve_locked_apm_yml_paths +from apm_cli.integration._shared import ( + deduplicate_deps, + resolve_locked_apm_yml_sources, +) +from apm_cli.integration.base_integrator import BaseIntegrator from apm_cli.runtime.utils import find_runtime_binary from apm_cli.utils.atomic_io import write_text_lf @@ -47,6 +50,9 @@ class _LSPTargetSpec: user_servers_key: str | None project_label: str user_label: str + project_config_defaults: tuple[tuple[str, str], ...] = () + user_config_defaults: tuple[tuple[str, str], ...] = () + cleanup_empty_relative_dirs: tuple[tuple[str, ...], ...] = () def path(self, project_root: Path, *, user_scope: bool) -> Path: """Return the config path for this target and scope.""" @@ -62,18 +68,54 @@ def label(self, *, user_scope: bool) -> str: """Return a human-readable config path label.""" return self.user_label if user_scope else self.project_label + def config_defaults(self, *, user_scope: bool) -> tuple[tuple[str, str], ...]: + """Return required top-level defaults for this target and scope.""" + return self.user_config_defaults if user_scope else self.project_config_defaults + + def cleanup_empty_dirs(self, project_root: Path, *, user_scope: bool) -> tuple[Path, ...]: + """Return APM-owned directories this target may remove when empty.""" + root = Path.home() if user_scope else project_root + return tuple(root.joinpath(*parts) for parts in self.cleanup_empty_relative_dirs) + + +@dataclass(frozen=True) +class _PreparedTargetConfig: + """Validated target config ready for one atomic file write.""" + + path: Path + content: str + changed: builtins.set + _LSP_TARGET_SPECS: dict[str, _LSPTargetSpec] = { "claude": _LSPTargetSpec( runtime="claude", - project_relative_path=(".lsp.json",), - user_relative_path=(".claude.json",), + project_relative_path=( + ".claude", + "skills", + "apm-lsp", + ".claude-plugin", + "plugin.json", + ), + user_relative_path=( + ".claude", + "skills", + "apm-lsp", + ".claude-plugin", + "plugin.json", + ), language_key=_CLAUDE_LANGUAGE_KEY, startup_timeout_key=_CLAUDE_STARTUP_TIMEOUT_KEY, - project_servers_key=None, + project_servers_key=_LSP_SERVERS_KEY, user_servers_key=_LSP_SERVERS_KEY, - project_label=".lsp.json", - user_label="~/.claude.json", + project_label=".claude/skills/apm-lsp/.claude-plugin/plugin.json", + user_label="~/.claude/skills/apm-lsp/.claude-plugin/plugin.json", + project_config_defaults=(("name", "apm-lsp"),), + user_config_defaults=(("name", "apm-lsp"),), + cleanup_empty_relative_dirs=( + (".claude", "skills", "apm-lsp", ".claude-plugin"), + (".claude", "skills", "apm-lsp"), + ), ), "copilot": _LSPTargetSpec( runtime="copilot", @@ -90,7 +132,7 @@ def label(self, *, user_scope: bool) -> str: class LSPIntegrator: - """LSP lifecycle orchestrator: dependency resolution, installation, and cleanup. + """Adapt runtime-neutral LSP declarations to target configuration files. All methods are static: the class is a logical namespace, not a stateful object. @@ -113,8 +155,8 @@ def collect_transitive( picking up stale/orphaned packages from previous installs. Falls back to scanning all apm.yml files if no lock file is available. - All LSP servers from installed packages are trusted (unlike MCP, - LSP has no registry vs self-defined distinction). + Declaring-package provenance is attached so the install pipeline can + enforce executable approval before exposing transitive servers. """ if logger is None: logger = NullCommandLogger() @@ -123,16 +165,42 @@ def collect_transitive( from apm_cli.models.apm_package import APMPackage - resolved, _ = resolve_locked_apm_yml_paths(apm_modules_dir, lock_path) - apm_yml_paths = resolved if resolved is not None else apm_modules_dir.rglob("apm.yml") + resolved, _ = resolve_locked_apm_yml_sources(apm_modules_dir, lock_path) + if resolved is None: + apm_yml_sources = [ + ( + path, + None, + ) + for path in apm_modules_dir.rglob("apm.yml") + ] + else: + apm_yml_sources = resolved collected = [] - for apm_yml_path in apm_yml_paths: + for apm_yml_path, locked_dependency in apm_yml_sources: try: pkg = APMPackage.from_apm_yml(apm_yml_path) lsp = pkg.get_lsp_dependencies() if lsp: - collected.extend(lsp) + if locked_dependency is None: + owner = apm_yml_path.parent.relative_to(apm_modules_dir).as_posix() + approval_keys: tuple[str, ...] = () + else: + from apm_cli.security.executables import ( + locked_dependency_approval_keys, + ) + + owner = locked_dependency.get_unique_key() + approval_keys = locked_dependency_approval_keys(locked_dependency) + collected.extend( + replace( + dependency, + resolved_by=owner, + approval_keys=approval_keys, + ) + for dependency in lsp + ) except Exception: _log.debug( "Skipping package at %s: failed to parse apm.yml", @@ -337,10 +405,47 @@ def resolve_target_runtimes( return [target for target in _LSP_TARGET_ORDER if target in target_runtimes] + @staticmethod + def supported_target_runtimes(target_runtimes: list[str]) -> list[str]: + """Return requested runtimes that have an LSP target adapter.""" + requested = set(target_runtimes) + return [target for target in _LSP_TARGET_ORDER if target in requested] + # ------------------------------------------------------------------ # JSON write helpers # ------------------------------------------------------------------ + @staticmethod + def reserved_project_skill_names(skills_dir: Path, project_root: Path) -> set[str]: + """Return LSP-owned names nested under one target's skills directory.""" + reserved: set[str] = set() + for spec in _LSP_TARGET_SPECS.values(): + parts = spec.project_relative_path + if len(parts) < 3 or parts[1] != "skills": + continue + if skills_dir == project_root.joinpath(*parts[:2]): + reserved.add(parts[2]) + return reserved + + @staticmethod + def _target_config_path( + spec: _LSPTargetSpec, + project_root: Path, + *, + user_scope: bool, + ) -> Path: + """Resolve a target config through the canonical deployment-path gate.""" + if user_scope: + user_root = Path.home() + relative_path = Path(*spec.user_relative_path).as_posix() + return BaseIntegrator.resolve_deploy_path( + relative_path, + user_root, + allowed_prefixes=(relative_path,), + ) + relative_path = Path(*spec.project_relative_path).as_posix() + return BaseIntegrator.resolve_deploy_path(relative_path, project_root) + @staticmethod def _read_json_object(config_path: Path, *, fail_on_error: bool = False) -> dict: """Read a JSON object from disk, returning an empty object on malformed input.""" @@ -352,19 +457,72 @@ def _read_json_object(config_path: Path, *, fail_on_error: bool = False) -> dict if fail_on_error: raise return {} - return data if isinstance(data, dict) else {} + if isinstance(data, dict): + return data + if fail_on_error: + raise ValueError(f"{config_path} must contain a JSON object") + return {} @staticmethod - def _write_target_config( + def _prepare_target_config( spec: _LSPTargetSpec, servers: dict[str, dict], *, project_root: Path, user_scope: bool, - ) -> builtins.set: - """Merge servers into one target config and return changed server names.""" - config_path = spec.path(project_root, user_scope=user_scope) - config = LSPIntegrator._read_json_object(config_path) + managed_server_names: builtins.set | None = None, + force: bool = False, + ) -> _PreparedTargetConfig: + """Validate and render one target config without writing it.""" + config_path = LSPIntegrator._target_config_path( + spec, + project_root, + user_scope=user_scope, + ) + managed_names = managed_server_names or builtins.set() + protected_plugin = bool(spec.config_defaults(user_scope=user_scope)) + skill_root = config_path.parent.parent + if protected_plugin and skill_root.exists() and not force: + foreign_entries = [ + entry.name for entry in skill_root.iterdir() if entry.name != ".claude-plugin" + ] + if config_path.parent.exists(): + foreign_entries.extend( + f".claude-plugin/{entry.name}" + for entry in config_path.parent.iterdir() + if entry != config_path + ) + if foreign_entries: + raise FileExistsError( + f"{spec.label(user_scope=user_scope)} shares its reserved skill " + f"directory with unowned content ({', '.join(sorted(foreign_entries))}); " + "move that content or rerun with --force" + ) + if ( + protected_plugin + and not managed_names + and skill_root.exists() + and not config_path.exists() + ): + if any(skill_root.iterdir()) and not force: + raise FileExistsError( + f"{spec.label(user_scope=user_scope)} is inside an existing " + "skill directory not owned by APM; remove it or rerun with --force" + ) + config_exists = config_path.exists() + config = LSPIntegrator._read_json_object( + config_path, + fail_on_error=protected_plugin and not force, + ) + for key, value in spec.config_defaults(user_scope=user_scope): + current_default = config.get(key) + if config_exists and current_default != value and not force: + raise FileExistsError( + f"{spec.label(user_scope=user_scope)} is owned by plugin " + f"'{current_default or '(unnamed)'}', not APM; remove it or " + "rerun with --force" + ) + config[key] = value servers_key = spec.servers_key(user_scope=user_scope) if servers_key is None: @@ -375,18 +533,65 @@ def _write_target_config( else: existing = config.get(servers_key, {}) if not isinstance(existing, dict): + if protected_plugin and not force: + raise ValueError( + f"{spec.label(user_scope=user_scope)} has a non-object " + f"'{servers_key}' value; repair it or rerun with --force" + ) existing = {} config[servers_key] = existing changed: builtins.set = builtins.set() for name, server_config in servers.items(): + if ( + protected_plugin + and name in existing + and existing[name] != server_config + and name not in managed_names + and not force + ): + raise FileExistsError( + f"LSP server '{name}' already exists in " + f"{spec.label(user_scope=user_scope)} and is not managed by APM; " + "rename it or rerun with --force" + ) if existing.get(name) != server_config: changed.add(name) existing[name] = server_config - config_path.parent.mkdir(parents=True, exist_ok=True) - write_text_lf(config_path, json.dumps(config, indent=2) + "\n") - return changed + return _PreparedTargetConfig( + path=config_path, + content=json.dumps(config, indent=2) + "\n", + changed=changed, + ) + + @staticmethod + def _write_prepared_target_config(prepared: _PreparedTargetConfig) -> builtins.set: + """Write one config after every target in the install has validated.""" + prepared.path.parent.mkdir(parents=True, exist_ok=True) + write_text_lf(prepared.path, prepared.content) + return prepared.changed + + @staticmethod + def _write_target_config( + spec: _LSPTargetSpec, + servers: dict[str, dict], + *, + project_root: Path, + user_scope: bool, + managed_server_names: builtins.set | None = None, + force: bool = False, + ) -> builtins.set: + """Validate, merge, and write one target config.""" + prepared = LSPIntegrator._prepare_target_config( + spec, + servers, + project_root=project_root, + user_scope=user_scope, + managed_server_names=managed_server_names, + force=force, + ) + return LSPIntegrator._write_prepared_target_config(prepared) @staticmethod def _clean_target_config( @@ -398,13 +603,22 @@ def _clean_target_config( fail_on_write_error: bool = False, ) -> list[str]: """Remove stale names from one target config and return removed names.""" - config_path = spec.path(project_root, user_scope=user_scope) + config_path = LSPIntegrator._target_config_path( + spec, + project_root, + user_scope=user_scope, + ) if not config_path.exists(): return [] config = LSPIntegrator._read_json_object( config_path, fail_on_error=fail_on_write_error, ) + for key, value in spec.config_defaults(user_scope=user_scope): + if config.get(key) != value: + if fail_on_write_error: + raise ValueError(f"{spec.label(user_scope=user_scope)} is not owned by APM") + return [] servers_key = spec.servers_key(user_scope=user_scope) servers = config if servers_key is None else config.get(servers_key, {}) @@ -417,7 +631,18 @@ def _clean_target_config( if removed: if servers_key is not None: config[servers_key] = servers - write_text_lf(config_path, json.dumps(config, indent=2) + "\n") + owned_keys = {key for key, _value in spec.config_defaults(user_scope=user_scope)} + if servers_key is not None: + owned_keys.add(servers_key) + if not servers and owned_keys and set(config) <= owned_keys: + config_path.unlink() + for directory in spec.cleanup_empty_dirs(project_root, user_scope=user_scope): + try: + directory.rmdir() + except OSError: + break + else: + write_text_lf(config_path, json.dumps(config, indent=2) + "\n") return removed # ------------------------------------------------------------------ @@ -454,10 +679,19 @@ def remove_stale( user_scope=user_scope, fail_on_write_error=fail_on_write_error, ) - for name in removed: + if removed: + noun = "server" if len(removed) == 1 else "servers" + removed_names = ", ".join(sorted(removed)) logger.progress( - f"Removed stale LSP server '{name}' from {spec.label(user_scope=user_scope)}" + f"Removed {len(removed)} stale LSP {noun} ({removed_names}) from " + f"{spec.label(user_scope=user_scope)}" ) + for name in removed: + logger.verbose_detail(f"Removed stale LSP server: {name}") + if runtime == "claude" and not user_scope: + logger.progress( + " |-- run /reload-plugins or restart Claude Code to activate" + ) except Exception as exc: _log.debug( "Failed to clean stale LSP servers from %s", @@ -468,8 +702,9 @@ def remove_stale( from apm_cli.install.errors import RequiredIntegrationError raise RequiredIntegrationError( - f"LSP cleanup failed for target '{runtime}'. " - "Check the target config path and permissions, then retry." + f"LSP cleanup failed for target '{runtime}' at " + f"{spec.label(user_scope=user_scope)}: {exc}. " + "Review the path and permissions, then retry." ) from exc # ------------------------------------------------------------------ @@ -482,19 +717,39 @@ def update_lockfile( lock_path: Path | None = None, *, lsp_configs: builtins.dict | None = None, + lsp_target_servers: dict[str, set[str]] | None = None, + lsp_config_provenance: dict[str, str] | None = None, + lockfile_state: LockFile | None = None, + persist: bool = True, fail_on_write_error: bool = False, ) -> None: """Update the lockfile with the current set of APM-managed LSP servers.""" - if lock_path is None: + if lock_path is None and persist: lock_path = get_lockfile_path(Path.cwd()) try: - lockfile = LockFile.read(lock_path) if lock_path.exists() else LockFile() + lockfile = lockfile_state + if lockfile is None and lock_path is not None: + lockfile = LockFile.read(lock_path) if lock_path.exists() else LockFile() if lockfile is None: lockfile = LockFile() lockfile.lsp_servers = sorted(lsp_server_names) if lsp_configs is not None: lockfile.lsp_configs = lsp_configs - lockfile.save(lock_path) + if lsp_config_provenance is not None: + lockfile.lsp_config_provenance = lsp_config_provenance + if lsp_target_servers is not None: + from apm_cli.core.deployment_ledger import DeploymentLedgerCodec + + DeploymentLedgerCodec.replace_lsp_target_servers( + lockfile, + { + runtime: sorted(names) + for runtime, names in lsp_target_servers.items() + if names + }, + ) + if persist and lock_path is not None: + lockfile.save(lock_path) except Exception as exc: _log.debug( "Failed to update LSP servers in lockfile at %s", @@ -509,7 +764,7 @@ def update_lockfile( ) from exc # ------------------------------------------------------------------ - # Main orchestrator + # Target deployment # ------------------------------------------------------------------ @staticmethod @@ -521,6 +776,9 @@ def install( diagnostics=None, target_runtimes: list[str] | None = None, fail_on_write_error: bool = False, + managed_server_names: builtins.set | None = None, + managed_target_servers: dict[str, set[str]] | None = None, + force: bool = False, ) -> int: """Install LSP dependencies by writing target-specific runtime config.""" if logger is None: @@ -539,23 +797,71 @@ def install( if not base_servers: return 0 - changed_servers: builtins.set = builtins.set() + prepared_targets: list[tuple[str, _LSPTargetSpec, _PreparedTargetConfig]] = [] for runtime in runtimes: spec = _LSP_TARGET_SPECS[runtime] servers = LSPIntegrator._servers_for_target(base_servers, spec) try: - changed = LSPIntegrator._write_target_config( + managed_names = managed_server_names + if managed_target_servers is not None: + managed_names = managed_target_servers.get(runtime, set()) + prepared = LSPIntegrator._prepare_target_config( spec, servers, project_root=project_root_path, user_scope=user_scope, + managed_server_names=managed_names, + force=force, ) + prepared_targets.append((runtime, spec, prepared)) + except Exception as exc: + _log.debug( + "Failed to write LSP config to %s", + spec.label(user_scope=user_scope), + exc_info=True, + ) + if diagnostics: + diagnostics.warn( + f"Failed to write LSP config to {spec.path(project_root_path, user_scope=user_scope)}: " + f"{exc}. Check file permissions or run with --verbose for details." + ) + if fail_on_write_error: + from apm_cli.install.errors import RequiredIntegrationError + + raise RequiredIntegrationError( + f"LSP configuration failed for target '{runtime}' at " + f"{spec.label(user_scope=user_scope)}: {exc}. " + "Review the path and permissions, then retry; use --force " + "only for a reviewed ownership collision." + ) from exc + + changed_servers: builtins.set = builtins.set() + for runtime, spec, prepared in prepared_targets: + try: + changed = LSPIntegrator._write_prepared_target_config(prepared) changed_servers.update(changed) if changed: + noun = "server" if len(changed) == 1 else "servers" logger.progress( - f"Configured {len(changed)} LSP server(s) in " + f"Configured {len(changed)} LSP {noun} in " f"{spec.label(user_scope=user_scope)}" ) + if runtime == "claude" and not user_scope: + logger.progress( + " |-- run /reload-plugins or restart Claude Code to activate" + ) + if runtime == "claude" and not user_scope: + legacy_path = project_root_path / ".lsp.json" + if legacy_path.exists(): + message = ( + "Retained legacy .lsp.json. Claude Code does not discover " + "project LSP servers there; review and remove it after " + "migrating any user-owned entries." + ) + if diagnostics is not None: + diagnostics.warn(message) + else: + logger.warning(message) except Exception as exc: _log.debug( "Failed to write LSP config to %s", @@ -564,15 +870,16 @@ def install( ) if diagnostics: diagnostics.warn( - f"Failed to write LSP config to {spec.path(project_root_path, user_scope=user_scope)}: " + f"Failed to write LSP config to {prepared.path}: " f"{exc}. Check file permissions or run with --verbose for details." ) if fail_on_write_error: from apm_cli.install.errors import RequiredIntegrationError raise RequiredIntegrationError( - f"LSP configuration failed for target '{runtime}'. " - "Check the target config path and permissions, then retry." + f"LSP configuration failed for target '{runtime}' at " + f"{spec.label(user_scope=user_scope)}: {exc}. " + "Review the path and permissions, then retry." ) from exc return len(changed_servers) diff --git a/src/apm_cli/integration/mcp_config_view.py b/src/apm_cli/integration/mcp_config_view.py index 559e71db9..2f776a8bf 100644 --- a/src/apm_cli/integration/mcp_config_view.py +++ b/src/apm_cli/integration/mcp_config_view.py @@ -12,7 +12,11 @@ from pathlib import Path from typing import TYPE_CHECKING, Any, Protocol -from apm_cli.deps.path_anchoring import LocalResolutionError, resolve_local_dep_dir +from apm_cli.deps.path_anchoring import ( + LocalResolutionError, + build_local_parent_index, + resolve_local_dep_dir, +) from apm_cli.integration._shared import deduplicate_deps from apm_cli.models.apm_package import APMPackage from apm_cli.models.dependency.mcp import MCPDependency @@ -197,10 +201,19 @@ def _package_manifest_path( lockfile: LockFile, modules_root: Path, project_root: Path, + *, + parent_index: dict[str, tuple[LockedDependency, ...]], + resolved_cache: dict[str, Path], ) -> Path: """Resolve a locked package's canonical current manifest path.""" if dependency.source == "local": - package_dir = resolve_local_dep_dir(dependency, lockfile, project_root) + package_dir = resolve_local_dep_dir( + dependency, + lockfile, + project_root, + parent_index=parent_index, + resolved_cache=resolved_cache, + ) else: package_dir = dependency.to_dependency_ref().get_install_path(modules_root) return (package_dir / "apm.yml").resolve() @@ -267,6 +280,8 @@ def _collect_locked_dependencies( collected: list[MCPDependency] = [] problems: list[McpSourceProblem] = [] + parent_index = build_local_parent_index(lockfile) + resolved_cache: dict[str, Path] = {} for package_key, dependency in lockfile.dependencies.items(): if package_key == ".": continue @@ -277,6 +292,8 @@ def _collect_locked_dependencies( lockfile, modules_root, project_root, + parent_index=parent_index, + resolved_cache=resolved_cache, ) except LocalResolutionError as exc: problems.append( diff --git a/src/apm_cli/integration/skill_support.py b/src/apm_cli/integration/skill_support.py index d225f2e87..4d901979b 100644 --- a/src/apm_cli/integration/skill_support.py +++ b/src/apm_cli/integration/skill_support.py @@ -56,6 +56,11 @@ def clean_orphaned_skills( get_lockfile_owned_agent_skills: Callable[[Path], set[str]], ) -> dict[str, int]: """Remove legacy-orphan skill directories without touching foreign agents.""" + protected_names = set(installed_skill_names) + if project_root is not None: + from apm_cli.integration.lsp_integrator import LSPIntegrator + + protected_names.update(LSPIntegrator.reserved_project_skill_names(skills_dir, project_root)) files_removed = 0 errors = 0 lockfile_owned_skills: set[str] | None = None @@ -63,7 +68,7 @@ def clean_orphaned_skills( lockfile_owned_skills = get_lockfile_owned_agent_skills(project_root) for skill_subdir in skills_dir.iterdir(): - if not skill_subdir.is_dir() or skill_subdir.name in installed_skill_names: + if not skill_subdir.is_dir() or skill_subdir.name in protected_names: continue if lockfile_owned_skills is not None and skill_subdir.name not in lockfile_owned_skills: continue diff --git a/src/apm_cli/models/dependency/lsp.py b/src/apm_cli/models/dependency/lsp.py index 04de14e15..16586ab20 100644 --- a/src/apm_cli/models/dependency/lsp.py +++ b/src/apm_cli/models/dependency/lsp.py @@ -43,6 +43,8 @@ class LSPDependency: shutdown_timeout: int | None = None restart_on_crash: bool | None = None max_restarts: int | None = None + resolved_by: str | None = None + approval_keys: tuple[str, ...] = () _VALID_TRANSPORTS = frozenset({"stdio", "socket"}) diff --git a/src/apm_cli/models/results.py b/src/apm_cli/models/results.py index 6e19b76de..c74fcda5b 100644 --- a/src/apm_cli/models/results.py +++ b/src/apm_cli/models/results.py @@ -35,6 +35,8 @@ class InstallResult: committed: bool = False error: BaseException | None = field(default=None, repr=False) target_decision: EffectiveTargetDecision | None = None + exec_allow_map: dict[str, dict[str, bool]] | None = None + exec_allow_resolved: bool = False @dataclass diff --git a/src/apm_cli/security/executables.py b/src/apm_cli/security/executables.py index 24bf4f937..d90c57773 100644 --- a/src/apm_cli/security/executables.py +++ b/src/apm_cli/security/executables.py @@ -18,6 +18,7 @@ from __future__ import annotations import fnmatch +import hashlib import os import sys from dataclasses import dataclass, field @@ -289,6 +290,11 @@ def _strip_version(package_key: str) -> str: return package_key.split("#", 1)[0] +def _is_content_bound_key(package_key: str) -> bool: + """Return whether an approval key identifies one exact content digest.""" + return "@sha256:" in package_key + + def normalize_bin_deploy_deny_key(value: object) -> str: """Normalize package identity for ``bin_deploy.deny`` storage and lookup.""" raw = str(value or "").strip() @@ -310,9 +316,8 @@ def _map_grants( ) -> bool: """Return True if *grant_map* grants *exec_type* for *package_key*. - Matches the exact key, the version-blind name, or any stored key that - shares the same version-blind name -- so approving ``owner/repo`` - covers ``owner/repo#v1`` and vice-versa. + Ordinary package grants are version-blind. Content-bound local-bundle + grants match only the exact digest key. """ if not grant_map: return False @@ -320,9 +325,13 @@ def _map_grants( for stored_key, entry in grant_map.items(): if not isinstance(entry, dict): continue - if (stored_key in (package_key, name) or _strip_version(stored_key) == name) and bool( - entry.get(exec_type, False) - ): + content_bound = _is_content_bound_key(package_key) or _is_content_bound_key(stored_key) + matches = ( + stored_key == package_key + if content_bound + else stored_key in (package_key, name) or _strip_version(stored_key) == name + ) + if matches and bool(entry.get(exec_type, False)): return True return False @@ -450,6 +459,57 @@ def build_approval_key(package_name: str, version: str) -> str: return f"{package_name}#{version}" +def locked_dependency_approval_keys(dependency: Any) -> tuple[str, ...]: + """Return trust keys derived only from a locked package identity.""" + identity = dependency.get_unique_key() + versioned = build_approval_key(identity, dependency.version or "") + return tuple(dict.fromkeys((identity, versioned))) + + +def local_bundle_approval_key( + package_id: str, + version: str, + source_dir: Path, + lockfile: dict[str, Any] | None = None, +) -> str: + """Bind local-bundle executable consent to every deployable content byte.""" + hasher = hashlib.sha256() + pack = lockfile.get("pack") if isinstance(lockfile, dict) else None + bundle_files = pack.get("bundle_files") if isinstance(pack, dict) else None + if isinstance(bundle_files, dict): + for rel_path in sorted(bundle_files, key=str): + digest = bundle_files[rel_path] + encoded_path = str(rel_path).encode("utf-8") + encoded_digest = str(digest).encode("utf-8") + hasher.update(len(encoded_path).to_bytes(8, "big")) + hasher.update(encoded_path) + hasher.update(len(encoded_digest).to_bytes(8, "big")) + hasher.update(encoded_digest) + else: + files: list[tuple[str, Path]] = [] + for item in source_dir.rglob("*"): + rel_path = item.relative_to(source_dir).as_posix() + if item.is_symlink(): + raise ValueError(f"Local bundle contains a symlink: {rel_path}") + if item.is_dir(): + continue + if not item.is_file(): + raise ValueError(f"Local bundle contains a special file: {rel_path}") + files.append((rel_path, item)) + for rel_path, item in sorted(files): + encoded_path = rel_path.encode("utf-8") + content_length = item.stat().st_size + hasher.update(len(encoded_path).to_bytes(8, "big")) + hasher.update(encoded_path) + hasher.update(content_length.to_bytes(8, "big")) + with item.open("rb") as handle: + while chunk := handle.read(1024 * 1024): + hasher.update(chunk) + artifact_version = version or "local" + digest = f"sha256:{hasher.hexdigest()}" + return f"{package_id}#{artifact_version}@{digest}" + + # ------------------------------------------------------------------- # Package scanning # ------------------------------------------------------------------- @@ -460,6 +520,8 @@ def scan_package_executables( package_name: str, package_version: str, *, + approval_identity: str | None = None, + manifest_data: dict[str, Any] | None = None, is_transitive: bool = False, parent_name: str | None = None, ) -> ExecutableDeclaration: @@ -477,7 +539,7 @@ def scan_package_executables( Returns an :class:`ExecutableDeclaration` (may have zero counts if the package declares no executables). """ - key = build_approval_key(package_name, package_version) + key = build_approval_key(approval_identity or package_name, package_version) # 1. Hooks: .apm/hooks/*.json and hooks/*.json (aligned with # HookIntegrator.find_hook_files -- only JSON files are actionable). @@ -514,32 +576,33 @@ def scan_package_executables( lsp_count = 0 lsp_details: list[str] = [] apm_yml = install_path / "apm.yml" - if apm_yml.is_file(): + data = manifest_data + if data is None and apm_yml.is_file(): try: from ..utils.yaml_io import load_yaml data = load_yaml(apm_yml) - if isinstance(data, dict): - deps = data.get("dependencies", {}) - if isinstance(deps, dict): - mcp_list = deps.get("mcp", []) - if isinstance(mcp_list, list): - mcp_count = len(mcp_list) - for entry in mcp_list: - if isinstance(entry, str): - mcp_details.append(entry) - elif isinstance(entry, dict): - mcp_details.append(entry.get("name", str(entry))) - lsp_list = deps.get("lsp", []) - if isinstance(lsp_list, list): - lsp_count = len(lsp_list) - for entry in lsp_list: - if isinstance(entry, str): - lsp_details.append(entry) - elif isinstance(entry, dict): - lsp_details.append(entry.get("name", str(entry))) except Exception: - pass # Non-fatal: if we cannot parse, treat as zero MCP + data = None + if isinstance(data, dict): + deps = data.get("dependencies", {}) + if isinstance(deps, dict): + mcp_list = deps.get("mcp", []) + if isinstance(mcp_list, list): + mcp_count = len(mcp_list) + for entry in mcp_list: + if isinstance(entry, str): + mcp_details.append(entry) + elif isinstance(entry, dict): + mcp_details.append(entry.get("name", str(entry))) + lsp_list = deps.get("lsp", []) + if isinstance(lsp_list, list): + lsp_count = len(lsp_list) + for entry in lsp_list: + if isinstance(entry, str): + lsp_details.append(entry) + elif isinstance(entry, dict): + lsp_details.append(entry.get("name", str(entry))) # 4. Canvas extensions: .apm/extensions//extension.mjs # Mirrors CanvasIntegrator.find_canvas_bundles marker detection. @@ -705,7 +768,7 @@ def parse_allow_executables(data: dict[str, Any]) -> dict[str, dict[str, bool]] if not isinstance(raw, dict): raise ValueError( "allowExecutables must be a mapping of " - "package keys to {hooks: bool, mcp: bool, bin: bool, canvas: bool}" + "package keys to {hooks: bool, mcp: bool, lsp: bool, bin: bool, canvas: bool}" ) result: dict[str, dict[str, bool]] = {} @@ -789,7 +852,7 @@ def materialize_exec_map(ctx: ExecTrustContext) -> dict[str, dict[str, bool]] | continue result.setdefault(key, {})[exec_type] = True name = _strip_version(key) - if name != key: + if name != key and not _is_content_bound_key(key): result.setdefault(name, {})[exec_type] = True return result @@ -851,6 +914,47 @@ def build_effective_exec_map( return materialize_exec_map(ctx) +def exec_trust_context_for_project( + project_root: Path, + *, + policy: Any | None, + fallback_allow_executables: dict[str, dict[str, bool]] | None = None, + logger: Any | None = None, +) -> ExecTrustContext: + """Resolve project, user, and policy executable trust through one owner.""" + from apm_cli.utils.yaml_io import load_yaml + + project_data: dict[str, Any] | None = None + manifest_path = project_root / "apm.yml" + if manifest_path.is_file(): + data = load_yaml(manifest_path) + if isinstance(data, dict): + project_data = data + if data.get("allowExecutables") is not None: + warn_allow_executables_alias_once(logger) + if project_data is None and isinstance(fallback_allow_executables, dict): + project_data = {"allowExecutables": fallback_allow_executables} + return build_exec_trust_context(policy=policy, project_data=project_data) + + +def effective_exec_map_for_project( + project_root: Path, + *, + policy: Any | None, + fallback_allow_executables: dict[str, dict[str, bool]] | None = None, + logger: Any | None = None, +) -> dict[str, dict[str, bool]] | None: + """Materialize the canonical trust context for one project.""" + return materialize_exec_map( + exec_trust_context_for_project( + project_root, + policy=policy, + fallback_allow_executables=fallback_allow_executables, + logger=logger, + ) + ) + + def effective_allow_executables( project_allow_executables: dict[str, dict[str, bool]] | None, ) -> dict[str, dict[str, bool]] | None: @@ -932,17 +1036,65 @@ def filter_mcp_by_allow_executables( def filter_lsp_by_allow_executables( lsp_deps: list, - project_allow_execs: dict | None, + effective_allow_execs: dict | None, logger: Any, ) -> list: - """Filter LSP deps not approved in allowExecutables.""" - return _filter_service_dependencies_by_allow_executables( - lsp_deps, - project_allow_execs, - logger, - exec_type=EXEC_TYPE_LSP, - service_label="LSP", - ) + """Filter transitive LSP deps by their declaring package's decision.""" + if effective_allow_execs is None or not lsp_deps: + return lsp_deps + filtered = [] + skipped: dict[str, bool] = {} + for dependency in lsp_deps: + owner = getattr(dependency, "resolved_by", None) + approval_keys = getattr(dependency, "approval_keys", ()) + if owner is None or any( + is_package_approved(effective_allow_execs, key, EXEC_TYPE_LSP) for key in approval_keys + ): + filtered.append(dependency) + continue + skipped[owner] = bool(approval_keys) + dependency_name = getattr(dependency, "name", "(unnamed)") + if approval_keys: + logger.verbose_detail( + f"Skipping LSP server '{dependency_name}' from '{owner}': " + f"executables not trusted. Run 'apm policy explain {owner}'; " + f"if policy permits, run 'apm approve {owner}'." + ) + else: + logger.verbose_detail( + f"Skipping LSP server '{dependency_name}' from '{owner}': package " + "identity cannot be verified without lock state. Run 'apm install' " + "to regenerate apm.lock.yaml, then approve the package." + ) + if len(filtered) < len(lsp_deps): + skipped_count = len(lsp_deps) - len(filtered) + noun = "server" if skipped_count == 1 else "servers" + owners = ", ".join(f"'{owner}'" for owner in sorted(skipped)) + known_owners = sorted(owner for owner, has_keys in skipped.items() if has_keys) + unlocked_owners = sorted(owner for owner, has_keys in skipped.items() if not has_keys) + remediation_parts = [] + if known_owners: + remediation_parts.append( + (f"Run 'apm policy explain {known_owners[0]}'; approve it only if policy permits.") + if len(known_owners) == 1 + else ( + "Run 'apm policy explain ' for each identified package; " + "approve only packages policy permits." + ) + ) + if unlocked_owners: + package_noun = "that package" if len(unlocked_owners) == 1 else "each package" + remediation_parts.append( + f"Run 'apm install' to regenerate apm.lock.yaml, then approve {package_noun}." + ) + remediation = " " + " ".join(remediation_parts) + package_clause = "declaring package is" if len(skipped) == 1 else "declaring packages are" + logger.warning( + f"Filtered {skipped_count} LSP {noun} from {owners}: " + f"{package_clause} not trusted yet.{remediation}", + symbol="warning", + ) + return filtered def read_bundle_allow_executables(apm_yml_path: Path, logger: Any) -> dict | None: @@ -987,7 +1139,7 @@ def _parse_grant_block( if not isinstance(raw, dict): raise ValueError( f"{where} must be a mapping of package keys to " - "{hooks: bool, mcp: bool, bin: bool, canvas: bool}" + "{hooks: bool, mcp: bool, lsp: bool, bin: bool, canvas: bool}" ) result: dict[str, dict[str, bool]] = {} for pkg_key, entry in raw.items(): diff --git a/tests/integration/test_architecture_lsp_plugin.py b/tests/integration/test_architecture_lsp_plugin.py new file mode 100644 index 000000000..cd229a4bf --- /dev/null +++ b/tests/integration/test_architecture_lsp_plugin.py @@ -0,0 +1,183 @@ +"""Architecture guardrails for the Claude LSP plugin owner.""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from scripts.architecture_linter.runner import run_selected_rules + +pytestmark = pytest.mark.component + +ROOT = Path(__file__).resolve().parents[2] + + +def test_lsp_owner_rules_pass() -> None: + """The live source must satisfy both registered LSP ownership rules.""" + report = run_selected_rules( + ROOT, + ( + "install-deployment-executable-trust-context", + "install-deployment-lsp-target-contract", + "install-deployment-lsp-lifecycle", + ), + ) + + assert report.exit_code == 2 + assert report.violations == () + assert report.failures == () + + +def test_claude_lsp_plugin_path_bypass_is_rejected() -> None: + """The LSP rule must reject a direct project-path write.""" + path = "src/apm_cli/integration/lsp_integrator.py" + source = (ROOT / path).read_text(encoding="utf-8") + old = "return BaseIntegrator.resolve_deploy_path(relative_path, project_root)" + assert old in source + mutated = source.replace(old, "return spec.path(project_root, user_scope=False)", 1) + + report = run_selected_rules( + ROOT, + ("install-deployment-lsp-target-contract",), + source_overrides={path: mutated}, + ) + + assert report.exit_code == 2 + assert {item.rule_id for item in report.violations} == { + "install-deployment-lsp-target-contract" + } + + +def test_user_lsp_config_path_bypass_is_rejected() -> None: + """The LSP rule must reject a direct user-config path write.""" + path = "src/apm_cli/integration/lsp_integrator.py" + source = (ROOT / path).read_text(encoding="utf-8") + old = "allowed_prefixes=(relative_path,)" + assert old in source + mutated = source.replace(old, "allowed_prefixes=()", 1) + + report = run_selected_rules( + ROOT, + ("install-deployment-lsp-target-contract",), + source_overrides={path: mutated}, + ) + + assert report.exit_code == 2 + assert {item.rule_id for item in report.violations} == { + "install-deployment-lsp-target-contract" + } + + +def test_lsp_target_ownership_bypass_is_rejected() -> None: + """The LSP rule must require target-scoped state at the lifecycle owner.""" + path = "src/apm_cli/install/lsp/integration.py" + source = (ROOT / path).read_text(encoding="utf-8") + old = "lsp_target_servers=new_targets" + assert old in source + mutated = source.replace(old, "lsp_target_servers={}") + + report = run_selected_rules( + ROOT, + ("install-deployment-lsp-lifecycle",), + source_overrides={path: mutated}, + ) + + assert report.exit_code == 2 + assert {item.rule_id for item in report.violations} == {"install-deployment-lsp-lifecycle"} + + +def test_lsp_lifecycle_direct_call_bypass_is_rejected() -> None: + path = "src/apm_cli/install/services.py" + source = (ROOT / path).read_text(encoding="utf-8") + mutated = source + "\nLSPIntegrator.install([])\n" + + report = run_selected_rules( + ROOT, + ("install-deployment-lsp-lifecycle",), + source_overrides={path: mutated}, + ) + + assert {item.rule_id for item in report.violations} == {"install-deployment-lsp-lifecycle"} + + +def test_claude_lsp_approval_alias_bypass_is_rejected() -> None: + """The LSP rule must reject local approval-key derivation.""" + path = "src/apm_cli/integration/lsp_integrator.py" + source = (ROOT / path).read_text(encoding="utf-8") + old = "locked_dependency_approval_keys(locked_dependency)" + assert old in source + mutated = source.replace(old, "(locked_dependency.name,)", 1) + + report = run_selected_rules( + ROOT, + ("install-deployment-lsp-target-contract",), + source_overrides={path: mutated}, + ) + + assert report.exit_code == 2 + assert {item.rule_id for item in report.violations} == { + "install-deployment-lsp-target-contract" + } + + +def test_local_bundle_content_approval_bypass_is_rejected() -> None: + """The executable-trust rule must require content-bound bundle consent.""" + path = "src/apm_cli/install/local_bundle_handler.py" + source = (ROOT / path).read_text(encoding="utf-8") + old = "bundle_approval_key = local_bundle_approval_key(" + assert old in source + mutated = source.replace(old, "bundle_approval_key = build_approval_key(", 1) + + report = run_selected_rules( + ROOT, + ("install-deployment-executable-trust-context",), + source_overrides={path: mutated}, + ) + + assert report.exit_code == 2 + assert {item.rule_id for item in report.violations} == { + "install-deployment-executable-trust-context" + } + + +def test_local_bundle_canvas_approval_bypass_is_rejected() -> None: + path = "src/apm_cli/install/services.py" + source = (ROOT / path).read_text(encoding="utf-8") + old = "is_package_approved(allow_executables, approval_key, EXEC_TYPE_CANVAS)" + assert old in source + mutated = source.replace( + old, + "is_package_approved(allow_executables, slug, EXEC_TYPE_CANVAS)", + 1, + ) + + report = run_selected_rules( + ROOT, + ("install-deployment-executable-trust-context",), + source_overrides={path: mutated}, + ) + + assert report.exit_code == 2 + assert {item.rule_id for item in report.violations} == { + "install-deployment-executable-trust-context" + } + + +def test_multitarget_lsp_preflight_bypass_is_rejected() -> None: + path = "src/apm_cli/integration/lsp_integrator.py" + source = (ROOT / path).read_text(encoding="utf-8") + old = "prepared_targets.append((runtime, spec, prepared))" + assert old in source + mutated = source.replace(old, "pass # removed multi-target preflight", 1) + + report = run_selected_rules( + ROOT, + ("install-deployment-lsp-target-contract",), + source_overrides={path: mutated}, + ) + + assert report.exit_code == 2 + assert {item.rule_id for item in report.violations} == { + "install-deployment-lsp-target-contract" + } diff --git a/tests/integration/test_architecture_owner_rule_mutations.py b/tests/integration/test_architecture_owner_rule_mutations.py index ea8c4a282..0ef94fd0a 100644 --- a/tests/integration/test_architecture_owner_rule_mutations.py +++ b/tests/integration/test_architecture_owner_rule_mutations.py @@ -6,10 +6,10 @@ every guard executes exactly once per run. Names prove nothing about teeth: a rule whose body was gutted still registers its guard ID and still runs. -This file supplies the missing half of that contract. For each of the 58 -registered owner guards it pins one minimal, meaningful source mutation -- a -surgical edit that kills a load-bearing sub-condition of the owning decision -- -and asserts the one rule that owns that guard reports a real `Violation`. +This file supplies the missing half of that contract. For each registered owner +guard it pins one minimal, meaningful source mutation -- a surgical edit that +kills a load-bearing sub-condition of the owning decision -- and asserts the one +rule that owns that guard reports a real `Violation`. Coverage is a set equality against the live registry, so a new owner guard that lands without a mutation case fails here instead of shipping a toothless rule. @@ -173,14 +173,6 @@ class MutationCase: new='import os\n\nos.environ.get("SOURCE_DATE_EPOCH")', intent="An Agent Plugin consumer reimplements the reproducible timestamp fallback.", ), - MutationCase( - guard_id="contracts-tooling-root-context-write-eligibility", - rule_id="contracts-tooling-root-context-write-eligibility", - path="src/apm_cli/compilation/agents_compiler.py", - old="and self._hand_authored_root_context_blocks_write(output_file)", - new="and False", - intent="Single-file compilation bypasses the root overwrite eligibility owner.", - ), MutationCase( guard_id="contracts-tooling-project-yaml-write-delegation", rule_id="contracts-tooling-project-yaml-write-delegation", @@ -189,6 +181,14 @@ class MutationCase: new=" write_text_lf(\n", intent="The atomic project YAML writer bypasses the canonical atomic writer.", ), + MutationCase( + guard_id="contracts-tooling-root-context-write-eligibility", + rule_id="contracts-tooling-root-context-write-eligibility", + path="src/apm_cli/compilation/agents_compiler.py", + old="def _hand_authored_root_context_blocks_write(", + new="def _hand_authored_root_context_blocks_write_disabled(", + intent="Root context writes lose the canonical hand-authored ownership gate.", + ), MutationCase( guard_id="hooks-integrations-copilot-cli-mcp-paths", rule_id="mutation_writes.copilot_cli_mcp_paths", @@ -277,6 +277,14 @@ class MutationCase: new=" def check_collision_disabled(", intent="BaseIntegrator drops a mandatory file-level deploy/sync/cleanup method.", ), + MutationCase( + guard_id="install-deployment-executable-trust-context", + rule_id="install-deployment-executable-trust-context", + path="src/apm_cli/security/executables.py", + old="def exec_trust_context_for_project(", + new="def exec_trust_context_for_project_disabled(", + intent="Executable trust loses its canonical project-context resolver.", + ), MutationCase( guard_id="install-deployment-frozen-mutation-eligibility", rule_id="install-deployment-frozen-mutation-eligibility", @@ -293,6 +301,22 @@ class MutationCase: new="user_scope=False", intent="Direct MCP target resolution stops consuming the command's scope decision.", ), + MutationCase( + guard_id="install-deployment-lsp-lifecycle", + rule_id="install-deployment-lsp-lifecycle", + path="src/apm_cli/install/lsp/integration.py", + old="def reconcile_lsp_after_uninstall(", + new="def reconcile_lsp_after_uninstall_disabled(", + intent="LSP reconciliation loses its canonical lifecycle entry point.", + ), + MutationCase( + guard_id="install-deployment-lsp-target-contract", + rule_id="install-deployment-lsp-target-contract", + path="src/apm_cli/integration/lsp_integrator.py", + old="return BaseIntegrator.resolve_deploy_path(relative_path, project_root)", + new="return spec.path(project_root, user_scope=False)", + intent="Claude LSP plugin writes bypass the canonical deployment-path gate.", + ), MutationCase( guard_id="install-deployment-mcp-ownership-migration", rule_id="install-deployment-mcp-ownership-migration", diff --git a/tests/integration/test_config_surface_lifecycle_contract.py b/tests/integration/test_config_surface_lifecycle_contract.py index c25c3179c..dfd9287d2 100644 --- a/tests/integration/test_config_surface_lifecycle_contract.py +++ b/tests/integration/test_config_surface_lifecycle_contract.py @@ -31,6 +31,8 @@ pytest.mark.requires_apm_binary, ] +_CLAUDE_LSP_PLUGIN = Path(".claude") / "skills" / "apm-lsp" / ".claude-plugin" / "plugin.json" + def _runner(apm_binary_path: Path) -> ApmLifecycleRunner: """Return the bounded real-binary runner for one lifecycle scenario.""" @@ -956,6 +958,349 @@ def test_lsp_reinstall_and_update_keep_copilot_state_deterministic( ) +def test_claude_lsp_install_writes_discoverable_skills_directory_plugin( + tmp_path: Path, + apm_binary_path: Path, +) -> None: + """Claude LSP install must emit the plugin manifest Claude discovers.""" + fixture = _create_git_lifecycle_project( + tmp_path / "claude-lsp-discovery", + source_name="claude-lsp-source", + lsp_dependencies=( + { + "name": "basedpyright", + "command": "uv", + "args": ["run", "basedpyright-langserver", "--stdio"], + "extensionToLanguage": {".py": "python", ".pyi": "python"}, + }, + ), + targets=("claude",), + ) + legacy_path = fixture.project_root / ".lsp.json" + legacy_bytes = b'{"lspServers":{"user-owned":{"command":"keep-me"}}}\n' + legacy_path.write_bytes(legacy_bytes) + + result = _runner(apm_binary_path).run_sequence( + (("install", "--no-policy"),), + expected_returncodes=(0,), + scenario_id="claude-lsp-discovery", + cwd=fixture.project_root, + env=fixture.isolated.subprocess_env(), + ) + + plugin_path = fixture.project_root / _CLAUDE_LSP_PLUGIN + assert json.loads(plugin_path.read_text(encoding="utf-8")) == { + "name": "apm-lsp", + "lspServers": { + "basedpyright": { + "command": "uv", + "args": ["run", "basedpyright-langserver", "--stdio"], + "extensionToLanguage": {".py": "python", ".pyi": "python"}, + } + }, + } + assert legacy_path.read_bytes() == legacy_bytes + assert "Retained legacy .lsp.json" in result[0].stdout + + +def test_claude_lsp_collision_requires_force_and_force_reconciles( + tmp_path: Path, + apm_binary_path: Path, +) -> None: + """The installed CLI must require explicit consent before replacement.""" + isolated = IsolatedApmEnvironment.create( + tmp_path / "claude-lsp-collision", + base_env=dict(os.environ), + ) + project = LocalPackageFactory(isolated.work_root).create( + "consumer", + lsp_dependencies=( + { + "name": "pyright", + "command": "pyright-langserver", + "extensionToLanguage": {".py": "python"}, + }, + ), + targets=("claude",), + ) + plugin_path = project.root / _CLAUDE_LSP_PLUGIN + plugin_path.parent.mkdir(parents=True) + original = b"[]\n" + plugin_path.write_bytes(original) + runner = _runner(apm_binary_path) + environment = isolated.subprocess_env() + + (refused,) = runner.run_sequence( + (("install", "--no-policy"),), + expected_returncodes=(1,), + scenario_id="claude-lsp-collision-refused", + cwd=project.root, + env=environment, + ) + assert "--force" in refused.stdout + assert plugin_path.read_bytes() == original + + runner.run_sequence( + (("install", "--no-policy", "--force"),), + expected_returncodes=(0,), + scenario_id="claude-lsp-collision-forced", + cwd=project.root, + env=environment, + ) + plugin = json.loads(plugin_path.read_text(encoding="utf-8")) + assert plugin["name"] == "apm-lsp" + assert set(plugin["lspServers"]) == {"pyright"} + + foreign = b'{"name":"custom-plugin","lspServers":{"pyright":{"command":"foreign"}}}\n' + plugin_path.write_bytes(foreign) + (update_refused,) = runner.run_sequence( + (("update", "--yes"),), + expected_returncodes=(1,), + scenario_id="claude-lsp-update-collision-refused", + cwd=project.root, + env=environment, + ) + assert "--force" in update_refused.stdout + assert plugin_path.read_bytes() == foreign + + runner.run_sequence( + (("update", "--yes", "--force"),), + expected_returncodes=(0,), + scenario_id="claude-lsp-update-collision-forced", + cwd=project.root, + env=environment, + ) + repaired = json.loads(plugin_path.read_text(encoding="utf-8")) + assert repaired["name"] == "apm-lsp" + assert repaired["lspServers"]["pyright"]["command"] == "pyright-langserver" + + +def test_claude_lsp_unapproved_package_is_not_discoverable( + tmp_path: Path, + apm_binary_path: Path, +) -> None: + """Executable approval must bind a generated LSP to its declaring package.""" + fixture = _create_git_lifecycle_project( + tmp_path / "claude-lsp-approval", + source_name="unapproved-lsp-source", + lsp_dependencies=( + { + "name": "shared-approved-name", + "command": "unapproved-language-server", + "extensionToLanguage": {".unsafe": "unsafe"}, + }, + ), + targets=("claude",), + ) + manifest_path = fixture.project_root / "apm.yml" + manifest = load_yaml(manifest_path) + manifest["executables"] = { + "allow": { + "different/package": { + "lsp": True, + } + } + } + dump_yaml(manifest, manifest_path) + + (result,) = _runner(apm_binary_path).run_sequence( + (("install", "--no-policy"),), + expected_returncodes=(0,), + scenario_id="claude-lsp-unapproved-package", + cwd=fixture.project_root, + env=fixture.isolated.subprocess_env(), + ) + + assert not (fixture.project_root / _CLAUDE_LSP_PLUGIN).exists() + assert "apm policy explain" in result.stdout + (partial,) = _runner(apm_binary_path).run_sequence( + (("install", "--only", "mcp", "--no-policy"),), + expected_returncodes=(0,), + scenario_id="claude-lsp-unapproved-package-partial-install", + cwd=fixture.project_root, + env=fixture.isolated.subprocess_env(), + ) + assert not (fixture.project_root / _CLAUDE_LSP_PLUGIN).exists() + assert "apm approve" not in partial.stdout + (updated,) = _runner(apm_binary_path).run_sequence( + (("update", "--yes"),), + expected_returncodes=(0,), + scenario_id="claude-lsp-unapproved-package-update", + cwd=fixture.project_root, + env=fixture.isolated.subprocess_env(), + ) + assert not (fixture.project_root / _CLAUDE_LSP_PLUGIN).exists() + assert "apm policy explain" in updated.stdout + + +def test_claude_lsp_approved_declaring_package_is_discoverable( + tmp_path: Path, + apm_binary_path: Path, +) -> None: + """A package-scoped LSP grant must materialize that package's server.""" + fixture = _create_git_lifecycle_project( + tmp_path / "claude-lsp-approved", + source_name="approved-lsp-source", + lsp_dependencies=( + { + "name": "server-name-is-not-the-approval-key", + "command": "approved-language-server", + "extensionToLanguage": {".safe": "safe"}, + }, + ), + targets=("claude",), + ) + manifest_path = fixture.project_root / "apm.yml" + manifest = load_yaml(manifest_path) + from apm_cli.models.apm_package import APMPackage + + approval_key = APMPackage.from_apm_yml(manifest_path).get_apm_dependencies()[0].get_unique_key() + manifest["executables"] = { + "allow": { + approval_key: { + "lsp": True, + } + } + } + dump_yaml(manifest, manifest_path) + + _runner(apm_binary_path).run_sequence( + (("install", "--no-policy"),), + expected_returncodes=(0,), + scenario_id="claude-lsp-approved-package", + cwd=fixture.project_root, + env=fixture.isolated.subprocess_env(), + ) + + plugin = json.loads((fixture.project_root / _CLAUDE_LSP_PLUGIN).read_text(encoding="utf-8")) + assert set(plugin["lspServers"]) == {"server-name-is-not-the-approval-key"} + + +def test_lsp_target_contraction_revokes_dropped_claude_plugin_entry( + tmp_path: Path, + apm_binary_path: Path, +) -> None: + """Changing the manifest target must revoke the old executable config.""" + fixture = _create_git_lifecycle_project( + tmp_path / "lsp-target-contraction", + source_name="target-contraction-source", + lsp_dependencies=( + { + "name": "target-contraction-lsp", + "command": "target-contraction-language-server", + "extensionToLanguage": {".target": "target"}, + }, + ), + targets=("claude",), + ) + runner = _runner(apm_binary_path) + environment = fixture.isolated.subprocess_env() + runner.run_sequence( + ((("install", "--no-policy")),), + expected_returncodes=(0,), + scenario_id="lsp-target-contraction-initial", + cwd=fixture.project_root, + env=environment, + ) + claude_lsp = fixture.project_root / _CLAUDE_LSP_PLUGIN + assert "target-contraction-lsp" in json.loads(claude_lsp.read_text())["lspServers"] + + manifest_path = fixture.project_root / "apm.yml" + manifest = load_yaml(manifest_path) + manifest["targets"] = ["copilot"] + dump_yaml(manifest, manifest_path) + runner.run_sequence( + ((("install", "--no-policy")),), + expected_returncodes=(0,), + scenario_id="lsp-target-contraction-copilot", + cwd=fixture.project_root, + env=environment, + ) + + copilot_lsp = fixture.project_root / ".github" / "lsp.json" + assert "target-contraction-lsp" in json.loads(copilot_lsp.read_text())["lspServers"] + assert not claude_lsp.exists() + lockfile = LockFile.read(fixture.project_root / "apm.lock.yaml") + assert lockfile is not None + assert lockfile.lsp_target_servers == {"copilot": ["target-contraction-lsp"]} + + +def test_only_mcp_does_not_materialize_root_lsp( + tmp_path: Path, + apm_binary_path: Path, +) -> None: + """The MCP-only filter must exclude root-project LSP declarations.""" + isolated = IsolatedApmEnvironment.create( + tmp_path / "only-mcp-lsp", + base_env=dict(os.environ), + ) + project = LocalPackageFactory(isolated.work_root).create( + "consumer", + lsp_dependencies=( + { + "name": "must-not-deploy", + "command": "must-not-run", + "extensionToLanguage": {".none": "none"}, + }, + ), + targets=("claude",), + ) + + _runner(apm_binary_path).run_sequence( + (("install", "--only", "mcp", "--no-policy"),), + expected_returncodes=(0,), + scenario_id="only-mcp-excludes-lsp", + cwd=project.root, + env=isolated.subprocess_env(), + ) + + assert not (project.root / _CLAUDE_LSP_PLUGIN).exists() + + +def test_lsp_uninstall_cleanup_failure_is_nonzero_and_actionable( + tmp_path: Path, + apm_binary_path: Path, +) -> None: + """Unsafe LSP cleanup must preserve foreign config and report recovery.""" + fixture = _create_git_lifecycle_project( + tmp_path / "lsp-uninstall-cleanup-failure", + source_name="lsp-uninstall-source", + lsp_dependencies=( + { + "name": "uninstall-lsp", + "command": "uninstall-language-server", + "extensionToLanguage": {".uninstall": "uninstall"}, + }, + ), + targets=("claude",), + ) + runner = _runner(apm_binary_path) + environment = fixture.isolated.subprocess_env() + runner.run_sequence( + (("install", "--no-policy"),), + expected_returncodes=(0,), + scenario_id="lsp-uninstall-cleanup-install", + cwd=fixture.project_root, + env=environment, + ) + plugin_path = fixture.project_root / _CLAUDE_LSP_PLUGIN + foreign = b'{"name":"foreign-plugin","lspServers":{"uninstall-lsp":{"command":"keep"}}}\n' + plugin_path.write_bytes(foreign) + + (result,) = runner.run_sequence( + (("uninstall", fixture.source),), + expected_returncodes=(1,), + scenario_id="lsp-uninstall-cleanup-refusal", + cwd=fixture.project_root, + env=environment, + ) + + assert plugin_path.read_bytes() == foreign + normalized_output = " ".join(result.stdout.split()) + assert "Uninstall incomplete" in normalized_output + assert "'apm install'" in normalized_output + + def test_saved_target_drives_package_mcp_lsp_update_audit_and_uninstall( tmp_path: Path, apm_binary_path: Path, @@ -996,12 +1341,14 @@ def test_saved_target_drives_package_mcp_lsp_update_audit_and_uninstall( env=environment, ) claude_mcp = fixture.project_root / ".mcp.json" - claude_lsp = fixture.project_root / ".lsp.json" + claude_lsp = fixture.project_root / _CLAUDE_LSP_PLUGIN + claude_lsp_plugin_dir = claude_lsp.parent + claude_lsp_dir = claude_lsp_plugin_dir.parent claude_instruction = ( fixture.project_root / ".claude" / "rules" / "saved-target-source-instruction.md" ) assert server_name in json.loads(claude_mcp.read_text(encoding="utf-8"))["mcpServers"] - assert lsp_name in json.loads(claude_lsp.read_text(encoding="utf-8")) + assert lsp_name in json.loads(claude_lsp.read_text(encoding="utf-8"))["lspServers"] assert claude_instruction.is_file() first_mcp = claude_mcp.read_bytes() @@ -1026,7 +1373,7 @@ def test_saved_target_drives_package_mcp_lsp_update_audit_and_uninstall( env=environment, ) assert server_name in json.loads(claude_mcp.read_text(encoding="utf-8"))["mcpServers"] - assert lsp_name in json.loads(claude_lsp.read_text(encoding="utf-8")) + assert lsp_name in json.loads(claude_lsp.read_text(encoding="utf-8"))["lspServers"] source_manifest = load_yaml(fixture.repository.worktree / "apm.yml") source_manifest["version"] = "0.2.0" @@ -1055,7 +1402,11 @@ def test_saved_target_drives_package_mcp_lsp_update_audit_and_uninstall( ) assert claude_instruction.is_file() assert server_name in json.loads(claude_mcp.read_text(encoding="utf-8"))["mcpServers"] - assert lsp_name in json.loads(claude_lsp.read_text(encoding="utf-8")) + assert lsp_name in json.loads(claude_lsp.read_text(encoding="utf-8"))["lspServers"] + unrelated_claude_skill = fixture.project_root / ".claude" / "skills" / "local-skill" + unrelated_claude_skill.mkdir(parents=True) + unrelated_skill_file = unrelated_claude_skill / "README.md" + unrelated_skill_file.write_text("local Claude skill\n", encoding="utf-8") runner.run_sequence( (install, ("audit", "--ci")), expected_returncodes=(0, 0), @@ -1065,13 +1416,20 @@ def test_saved_target_drives_package_mcp_lsp_update_audit_and_uninstall( ) runner.run_sequence( - (("uninstall", fixture.source), install), - expected_returncodes=(0, 0), + (("uninstall", fixture.source),), + expected_returncodes=(0,), scenario_id="saved-target-uninstall-reconcile", cwd=fixture.project_root, env=environment, ) assert not claude_instruction.exists() + post_uninstall_lock = LockFile.read(fixture.project_root / "apm.lock.yaml") + assert not claude_lsp.exists(), ( + post_uninstall_lock.lsp_config_provenance if post_uninstall_lock is not None else None + ) + assert not claude_lsp_plugin_dir.exists() + assert not claude_lsp_dir.exists() + assert unrelated_skill_file.read_text(encoding="utf-8") == "local Claude skill\n" runner.run_sequence( ( @@ -1144,10 +1502,23 @@ def test_saved_target_drives_user_scope_package_mcp_and_lsp( env=environment, ) claude_config = json.loads((isolated.home / ".claude.json").read_text(encoding="utf-8")) + claude_lsp_plugin = json.loads((isolated.home / _CLAUDE_LSP_PLUGIN).read_text(encoding="utf-8")) assert "saved-target-user-mcp" in claude_config["mcpServers"] - assert "saved-target-user-lsp" in claude_config["lspServers"] + assert "saved-target-user-lsp" in claude_lsp_plugin["lspServers"] + assert claude_lsp_plugin["name"] == "apm-lsp" assert (isolated.home / ".claude" / "rules" / "saved-target-user-instruction.md").is_file() + runner.run_sequence( + (("uninstall", "--global", str(package.root)),), + expected_returncodes=(0,), + scenario_id="saved-target-user-uninstall", + cwd=cwd, + env=environment, + ) + + assert not (isolated.home / _CLAUDE_LSP_PLUGIN).exists() + assert not (isolated.home / ".claude" / "skills" / "apm-lsp").exists() + def test_saved_target_drives_direct_mcp_without_target_flag( tmp_path: Path, @@ -1825,10 +2196,10 @@ def test_saved_target_drives_declared_mcp_and_lsp_without_package( ) assert (project.root / ".mcp.json").is_file() - assert (project.root / ".lsp.json").is_file() + assert (project.root / _CLAUDE_LSP_PLUGIN).is_file() (project.root / ".mcp.json").unlink() - (project.root / ".lsp.json").unlink() + (project.root / _CLAUDE_LSP_PLUGIN).unlink() runner.run_sequence( (("update", "--yes"),), expected_returncodes=(0,), @@ -1837,7 +2208,7 @@ def test_saved_target_drives_declared_mcp_and_lsp_without_package( env=environment, ) assert (project.root / ".mcp.json").is_file() - assert (project.root / ".lsp.json").is_file() + assert (project.root / _CLAUDE_LSP_PLUGIN).is_file() manifest = load_yaml(project.manifest_path) manifest["dependencies"].pop("mcp") @@ -1854,9 +2225,7 @@ def test_saved_target_drives_declared_mcp_and_lsp_without_package( "saved-declared-mcp" not in json.loads((project.root / ".mcp.json").read_text(encoding="utf-8"))["mcpServers"] ) - assert "saved-declared-lsp" not in json.loads( - (project.root / ".lsp.json").read_text(encoding="utf-8") - ) + assert not (project.root / _CLAUDE_LSP_PLUGIN).exists() def test_saved_copilot_target_projects_to_copilot_for_direct_mcp( @@ -2053,7 +2422,7 @@ def test_lsp_write_failure_is_nonzero( }, ), ) - (project.root / ".lsp.json").mkdir() + (project.root / _CLAUDE_LSP_PLUGIN).mkdir(parents=True) runner = _runner(apm_binary_path) environment = isolated.subprocess_env() runner.run_sequence( @@ -2121,7 +2490,7 @@ def test_manifest_and_explicit_target_precedence_for_mcp_and_lsp( assert (project.root / ".github" / "mcp.json").is_file() assert (project.root / ".github" / "lsp.json").is_file() assert not (project.root / ".mcp.json").exists() - assert not (project.root / ".lsp.json").exists() + assert not (project.root / _CLAUDE_LSP_PLUGIN).exists() runner.run_sequence( (("install", "--target", "claude", "--no-policy"),), @@ -2131,7 +2500,7 @@ def test_manifest_and_explicit_target_precedence_for_mcp_and_lsp( env=environment, ) assert (project.root / ".mcp.json").is_file() - assert (project.root / ".lsp.json").is_file() + assert (project.root / _CLAUDE_LSP_PLUGIN).is_file() def test_multi_target_exclusion_applies_to_mcp_and_lsp( @@ -2182,7 +2551,7 @@ def test_multi_target_exclusion_applies_to_mcp_and_lsp( ) assert (project.root / ".mcp.json").is_file() - assert (project.root / ".lsp.json").is_file() + assert (project.root / _CLAUDE_LSP_PLUGIN).is_file() assert not (project.root / ".vscode" / "mcp.json").exists() assert not (project.root / ".github" / "lsp.json").exists() @@ -2230,7 +2599,7 @@ def test_explicit_runtime_alias_overrides_saved_target_for_all_phases( ) assert (project.root / ".mcp.json").is_file() - assert (project.root / ".lsp.json").is_file() + assert (project.root / _CLAUDE_LSP_PLUGIN).is_file() assert not (project.root / ".vscode" / "mcp.json").exists() assert not (project.root / ".github" / "lsp.json").exists() @@ -2315,7 +2684,7 @@ def test_frozen_failure_writes_no_package_or_service_state( assert not (fixture.project_root / "apm_modules").exists() assert not (fixture.project_root / ".claude").exists() assert not (fixture.project_root / ".mcp.json").exists() - assert not (fixture.project_root / ".lsp.json").exists() + assert not (fixture.project_root / _CLAUDE_LSP_PLUGIN).exists() def test_unresolved_package_services_fail_before_package_deployment( @@ -2359,7 +2728,7 @@ def test_unresolved_package_services_fail_before_package_deployment( modules = fixture.project_root / "apm_modules" assert not modules.exists() or not any(modules.rglob("*")) assert not (fixture.project_root / ".mcp.json").exists() - assert not (fixture.project_root / ".lsp.json").exists() + assert not (fixture.project_root / _CLAUDE_LSP_PLUGIN).exists() def test_uninstalling_one_shared_root_retains_shared_dependency_ownership( diff --git a/tests/integration/test_install_local_bundle_e2e.py b/tests/integration/test_install_local_bundle_e2e.py index bdc82e0c9..c76103b2b 100644 --- a/tests/integration/test_install_local_bundle_e2e.py +++ b/tests/integration/test_install_local_bundle_e2e.py @@ -1209,6 +1209,399 @@ def test_multi_target_consumer_deploys_to_both( assert (project / ".agents" / "skills" / "coding" / "SKILL.md").is_file() +# --------------------------------------------------------------------------- +# E2E: bundle LSP wiring through the owned lifecycle +# --------------------------------------------------------------------------- + + +class TestInstallLocalBundleLsp: + """Local-bundle LSP writes must preserve consent and owner lifecycle.""" + + @staticmethod + def _bundle(tmp_path: Path, *, include_skill: bool = False) -> Path: + lsp_json = json.dumps( + { + "lspServers": { + "bundle-lsp": { + "command": "bundle-language-server", + "extensionToLanguage": {".bundle": "bundle"}, + } + } + } + ) + files = {"lsp.json": lsp_json} + if include_skill: + files["skills/coding/SKILL.md"] = "# Coding Skill\n" + return _make_plugin_bundle(tmp_path, files=files) + + @staticmethod + def _approve_exact_bundle(project: Path, bundle: Path) -> str: + from apm_cli.bundle.local_bundle import detect_local_bundle + from apm_cli.security.executables import local_bundle_approval_key + + bundle_info = detect_local_bundle(bundle) + assert bundle_info is not None + approval_key = local_bundle_approval_key( + bundle_info.package_id, + str(bundle_info.plugin_json.get("version") or ""), + bundle_info.source_dir, + bundle_info.lockfile, + ) + manifest_path = project / "apm.yml" + manifest = yaml.safe_load(manifest_path.read_text(encoding="utf-8")) + manifest["executables"] = {"allow": {approval_key: {"lsp": True}}} + manifest_path.write_text( + yaml.safe_dump(manifest, sort_keys=False), + encoding="utf-8", + ) + return approval_key + + def test_generic_identity_cannot_approve_changed_bundle_content( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + bundle = self._bundle(tmp_path / "source") + project = _make_project(tmp_path / "consumer", targets=["claude"]) + manifest_path = project / "apm.yml" + manifest = yaml.safe_load(manifest_path.read_text(encoding="utf-8")) + manifest["executables"] = {"allow": {"test-plugin": {"lsp": True}}} + manifest_path.write_text( + yaml.safe_dump(manifest, sort_keys=False), + encoding="utf-8", + ) + + result = _invoke_install( + project, + str(bundle), + "--target", + "claude", + monkeypatch=monkeypatch, + ) + + assert result.exit_code == 0, result.output + normalized_output = " ".join(result.output.split()) + assert "approve this exact local bundle" in normalized_output + assert "test-plugin#local@sha256:" in normalized_output + assert not ( + project / ".claude" / "skills" / "apm-lsp" / ".claude-plugin" / "plugin.json" + ).exists() + + def test_exact_bundle_digest_approval_enables_lsp( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + bundle = self._bundle(tmp_path / "source") + project = _make_project(tmp_path / "consumer", targets=["claude"]) + approval_key = self._approve_exact_bundle(project, bundle) + + result = _invoke_install( + project, + str(bundle), + "--target", + "claude", + monkeypatch=monkeypatch, + ) + + assert result.exit_code == 0, result.output + assert approval_key not in result.output + plugin_path = project / ".claude" / "skills" / "apm-lsp" / ".claude-plugin" / "plugin.json" + assert "bundle-lsp" in json.loads(plugin_path.read_text())["lspServers"] + + def test_unsupported_target_refuses_bundle_lsp( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + bundle = self._bundle(tmp_path / "source", include_skill=True) + project = _make_project(tmp_path / "consumer", targets=["cursor"]) + + result = _invoke_install( + project, + str(bundle), + "--target", + "cursor", + "--no-policy", + monkeypatch=monkeypatch, + ) + + assert result.exit_code == 1, result.output + assert "no LSP-compatible runtime" in result.output + assert not ( + project / ".claude" / "skills" / "apm-lsp" / ".claude-plugin" / "plugin.json" + ).exists() + assert not (project / ".agents" / "skills" / "coding" / "SKILL.md").exists() + lockfile = LockFile.read(project / "apm.lock.yaml") + assert lockfile is None or "cursor" not in lockfile.lsp_target_servers + + def test_all_targets_record_only_lsp_compatible_bundle_targets( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + bundle = self._bundle(tmp_path / "source") + project = _make_project(tmp_path / "consumer", targets=["all"]) + + result = _invoke_install( + project, + str(bundle), + "--target", + "all", + "--no-policy", + monkeypatch=monkeypatch, + ) + + assert result.exit_code == 0, result.output + lockfile = LockFile.read(project / "apm.lock.yaml") + assert lockfile is not None + assert set(lockfile.lsp_target_servers) <= {"claude", "copilot"} + assert "bundle-lsp" in lockfile.lsp_servers + + def test_root_install_reads_source_policy_for_bundle_lsp( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + bundle = self._bundle(tmp_path / "source") + project = _make_project(tmp_path / "consumer", targets=["claude"]) + manifest_path = project / "apm.yml" + manifest = yaml.safe_load(manifest_path.read_text(encoding="utf-8")) + manifest["executables"] = {"allow": {}} + manifest_path.write_text( + yaml.safe_dump(manifest, sort_keys=False), + encoding="utf-8", + ) + + result = _invoke_install( + project, + str(bundle), + "--root", + "out", + "--target", + "claude", + monkeypatch=monkeypatch, + ) + + assert result.exit_code == 0, result.output + assert "Skipped 1 bundle LSP executable" in result.output + assert not ( + project / "out" / ".claude" / "skills" / "apm-lsp" / ".claude-plugin" / "plugin.json" + ).exists() + + def test_root_install_reads_source_policy_for_bundle_integrity( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + from apm_cli.policy.discovery import PolicyFetchResult + from apm_cli.policy.schema import ApmPolicy, IntegrityPolicy, SecurityPolicy + + bundle = _make_plugin_bundle( + tmp_path / "source", + files={}, + include_lockfile=False, + ) + project = _make_project(tmp_path / "consumer", targets=["claude"]) + policy = ApmPolicy(security=SecurityPolicy(integrity=IntegrityPolicy(require_hashes=True))) + + def discover_policy(project_root: Path, **_kwargs) -> PolicyFetchResult: + if project_root == project.resolve(): + return PolicyFetchResult(policy=policy, source="test", outcome="found") + return PolicyFetchResult(policy=None, source="", outcome="absent") + + with patch( + "apm_cli.policy.install_preflight.discover_policy_with_chain", + side_effect=discover_policy, + ): + result = _invoke_install( + project, + str(bundle), + "--root", + "out", + "--target", + "claude", + monkeypatch=monkeypatch, + ) + + assert result.exit_code == 1, result.output + assert "requires integrity hashes" in result.output + + def test_symlinked_lsp_metadata_directory_is_ignored( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + bundle = _make_plugin_bundle( + tmp_path / "source", + files={}, + include_lockfile=False, + ) + external = tmp_path / "external-metadata" + external.mkdir() + (external / "lsp.json").write_text( + json.dumps( + { + "lspServers": { + "evil-lsp": { + "command": "evil-language-server", + "extensionToLanguage": {".evil": "evil"}, + } + } + } + ), + encoding="utf-8", + ) + namespace = bundle / "com.microsoft.apm" + try: + namespace.symlink_to(external, target_is_directory=True) + except OSError as exc: + pytest.skip(f"directory symlinks unavailable: {exc}") + project = _make_project(tmp_path / "symlink-consumer", targets=["claude"]) + + result = _invoke_install( + project, + str(bundle), + "--target", + "claude", + "--no-policy", + monkeypatch=monkeypatch, + ) + + assert result.exit_code == 0, result.output + assert not ( + project / ".claude" / "skills" / "apm-lsp" / ".claude-plugin" / "plugin.json" + ).exists() + + def test_org_deny_overrides_exact_bundle_lsp_approval( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + from apm_cli.policy.discovery import PolicyFetchResult + from apm_cli.policy.schema import ApmPolicy, ExecutablesPolicy + + bundle = self._bundle(tmp_path / "source") + project = _make_project(tmp_path / "consumer", targets=["claude"]) + self._approve_exact_bundle(project, bundle) + policy = ApmPolicy(executables=ExecutablesPolicy(deny=("test-plugin",))) + policy_fetch = PolicyFetchResult(policy=policy, source="test", outcome="found") + + with ( + patch( + "apm_cli.policy.discovery.discover_policy_with_chain", + return_value=policy_fetch, + ), + patch( + "apm_cli.policy.install_preflight.discover_policy_with_chain", + return_value=policy_fetch, + ), + ): + result = _invoke_install( + project, + str(bundle), + "--target", + "claude", + monkeypatch=monkeypatch, + ) + + assert result.exit_code == 0, result.output + assert "Skipped 1 bundle LSP executable" in result.output + assert not ( + project / ".claude" / "skills" / "apm-lsp" / ".claude-plugin" / "plugin.json" + ).exists() + + def test_copilot_aliases_are_normalized_for_bundle_lsp( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + lsp_json = json.dumps( + { + "lspServers": { + "bundle-lsp": { + "command": "bundle-language-server", + "fileExtensions": {".bundle": "bundle"}, + "warmupTimeoutMs": 1234, + } + } + } + ) + bundle = _make_plugin_bundle( + tmp_path / "source", + files={"lsp.json": lsp_json}, + ) + project = _make_project(tmp_path / "consumer", targets=["claude"]) + + result = _invoke_install( + project, + str(bundle), + "--target", + "claude", + monkeypatch=monkeypatch, + ) + + assert result.exit_code == 0, result.output + plugin_path = project / ".claude" / "skills" / "apm-lsp" / ".claude-plugin" / "plugin.json" + server = json.loads(plugin_path.read_text())["lspServers"]["bundle-lsp"] + assert server["extensionToLanguage"] == {".bundle": "bundle"} + assert server["startupTimeout"] == 1234 + + def test_lsp_collision_force_reconciles( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Foreign plugin content survives unless the operator passes --force.""" + bundle = self._bundle(tmp_path / "source") + project = _make_project(tmp_path / "consumer", targets=["claude"]) + plugin_path = project / ".claude" / "skills" / "apm-lsp" / ".claude-plugin" / "plugin.json" + plugin_path.parent.mkdir(parents=True) + foreign = b'{"name":"apm-lsp","lspServers":{"bundle-lsp":{"command":"foreign"}}}\n' + plugin_path.write_bytes(foreign) + + refused = _invoke_install( + project, + str(bundle), + "--target", + "claude", + monkeypatch=monkeypatch, + ) + + assert refused.exit_code != 0 + assert plugin_path.read_bytes() == foreign + + forced = _invoke_install( + project, + str(bundle), + "--target", + "claude", + "--force", + monkeypatch=monkeypatch, + ) + + assert forced.exit_code == 0, forced.output + plugin = json.loads(plugin_path.read_text(encoding="utf-8")) + assert plugin["name"] == "apm-lsp" + assert plugin["lspServers"]["bundle-lsp"]["command"] == "bundle-language-server" + + def test_explicit_deny_revokes_previously_owned_lsp( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Reinstalling under a closed gate removes the bundle's old command.""" + bundle = self._bundle(tmp_path / "source") + project = _make_project(tmp_path / "consumer", targets=["claude"]) + plugin_path = project / ".claude" / "skills" / "apm-lsp" / ".claude-plugin" / "plugin.json" + + installed = _invoke_install( + project, + str(bundle), + "--target", + "claude", + monkeypatch=monkeypatch, + ) + assert installed.exit_code == 0, installed.output + assert "bundle-lsp" in json.loads(plugin_path.read_text())["lspServers"] + + manifest = yaml.safe_load((project / "apm.yml").read_text(encoding="utf-8")) + manifest["allowExecutables"] = {} + (project / "apm.yml").write_text( + yaml.safe_dump(manifest, sort_keys=False), + encoding="utf-8", + ) + + denied = _invoke_install( + project, + str(bundle), + "--target", + "claude", + monkeypatch=monkeypatch, + ) + + assert denied.exit_code == 0, denied.output + assert not plugin_path.exists() + + # --------------------------------------------------------------------------- # E2E: bundle .mcp.json wiring through MCPIntegrator # --------------------------------------------------------------------------- diff --git a/tests/integration/test_integration_runtime_coverage.py b/tests/integration/test_integration_runtime_coverage.py index 4d395c8f4..665873e45 100644 --- a/tests/integration/test_integration_runtime_coverage.py +++ b/tests/integration/test_integration_runtime_coverage.py @@ -364,7 +364,9 @@ def test_project_path(self, tmp_path: Path) -> None: spec = _LSP_TARGET_SPECS["claude"] path = spec.path(tmp_path, user_scope=False) - assert path == tmp_path / ".lsp.json" + assert path == ( + tmp_path / ".claude" / "skills" / "apm-lsp" / ".claude-plugin" / "plugin.json" + ) def test_user_scope_label(self) -> None: """User-scope label differs from project-scope label.""" @@ -373,12 +375,23 @@ def test_user_scope_label(self) -> None: spec = _LSP_TARGET_SPECS["copilot"] assert spec.label(user_scope=True) != spec.label(user_scope=False) + def test_claude_user_scope_uses_personal_skills_plugin(self, tmp_path: Path) -> None: + """Claude user LSP config uses the documented personal skills plugin.""" + from apm_cli.integration.lsp_integrator import _LSP_TARGET_SPECS + + spec = _LSP_TARGET_SPECS["claude"] + assert spec.path(tmp_path, user_scope=True) == ( + Path.home() / ".claude" / "skills" / "apm-lsp" / ".claude-plugin" / "plugin.json" + ) + assert spec.label(user_scope=True) == "~/.claude/skills/apm-lsp/.claude-plugin/plugin.json" + assert ("name", "apm-lsp") in spec.config_defaults(user_scope=True) + def test_servers_key_project_scope(self) -> None: - """Claude project scope has None servers_key (top-level map).""" + """Claude project plugin manifest uses the lspServers wrapper key.""" from apm_cli.integration.lsp_integrator import _LSP_TARGET_SPECS spec = _LSP_TARGET_SPECS["claude"] - assert spec.servers_key(user_scope=False) is None + assert spec.servers_key(user_scope=False) == "lspServers" def test_servers_key_user_scope(self) -> None: """Claude user scope uses lspServers wrapper key.""" @@ -450,8 +463,8 @@ def test_write_no_change_on_identical(self, tmp_path: Path) -> None: ) assert len(changed2) == 0 - def test_write_claude_project_no_wrapper(self, tmp_path: Path) -> None: - """Claude project scope writes to top-level (no servers_key wrapper).""" + def test_write_claude_project_plugin_manifest(self, tmp_path: Path) -> None: + """Claude project scope writes a discoverable plugin manifest.""" from apm_cli.integration.lsp_integrator import _LSP_TARGET_SPECS, LSPIntegrator spec = _LSP_TARGET_SPECS["claude"] @@ -459,8 +472,8 @@ def test_write_claude_project_no_wrapper(self, tmp_path: Path) -> None: LSPIntegrator._write_target_config(spec, servers, project_root=tmp_path, user_scope=False) config_path = spec.path(tmp_path, user_scope=False) data = json.loads(config_path.read_text(encoding="utf-8")) - assert "python" in data - assert "lspServers" not in data + assert data["name"] == "apm-lsp" + assert "python" in data["lspServers"] class TestLSPCleanTargetConfig: diff --git a/tests/integration/test_lifecycle_state_snapshot_contract.py b/tests/integration/test_lifecycle_state_snapshot_contract.py index 3c76b652b..80c5fc123 100644 --- a/tests/integration/test_lifecycle_state_snapshot_contract.py +++ b/tests/integration/test_lifecycle_state_snapshot_contract.py @@ -121,7 +121,7 @@ def test_capture_preserves_raw_bytes_and_canonical_semantics(tmp_path: Path) -> assert snapshot.lsp_state_bytes == ( b'{"configs":{"fixture-lsp":{"command":"fixture-lsp",' b'"extensionToLanguage":{".py":"python"},"name":"fixture-lsp"}},' - b'"servers":["fixture-lsp"]}' + b'"provenance":{},"servers":["fixture-lsp"],"target_servers":{}}' ) assert snapshot.file(".agents/skills/review/SKILL.md").content == deployed_bytes assert snapshot.file(".agents/skills/review/SKILL.md").roles == frozenset({"deployment"}) diff --git a/tests/test_lockfile.py b/tests/test_lockfile.py index 6a9d646bf..9bb105ee6 100644 --- a/tests/test_lockfile.py +++ b/tests/test_lockfile.py @@ -533,6 +533,27 @@ def test_lsp_configs_empty_by_default(self): yaml_str = lock.to_yaml() assert "lsp_configs" not in yaml_str + def test_lsp_target_servers_round_trip_through_deployment_ledger(self, tmp_path): + """Target-scoped LSP ownership must survive lockfile serialization.""" + from apm_cli.core.deployment_ledger import DeploymentLedgerCodec + + lock = LockFile() + DeploymentLedgerCodec.replace_lsp_target_servers( + lock, + {"claude": ["pyright"], "copilot": ["ruff-lsp", "pyright"]}, + ) + lock_path = tmp_path / "apm.lock" + lock.write(lock_path) + + loaded = LockFile.read(lock_path) + + assert loaded is not None + assert loaded.lsp_target_servers == { + "claude": ["pyright"], + "copilot": ["pyright", "ruff-lsp"], + } + assert loaded._lsp_target_servers_present is True + def test_read_nonexistent(self, tmp_path): loaded = LockFile.read(tmp_path / "apm.lock.yaml") assert loaded is None diff --git a/tests/unit/commands/test_approve_deny.py b/tests/unit/commands/test_approve_deny.py index 72c89c504..2f4d028d7 100644 --- a/tests/unit/commands/test_approve_deny.py +++ b/tests/unit/commands/test_approve_deny.py @@ -19,6 +19,7 @@ approve_cmd, deny_cmd, load_org_policy, + scan_installed_executable_packages, ) from apm_cli.commands.policy import policy as policy_group from apm_cli.core.command_logger import CommandLogger @@ -58,6 +59,52 @@ def _create_pkg_with_bin(apm_modules: Path, name: str) -> None: (pkg_dir / "apm.yml").write_text(yaml.dump({"name": name, "version": "2.0"})) +def test_approval_scanner_rejects_symlinked_package_manifest(tmp_path: Path) -> None: + manifest = _write_manifest(str(tmp_path)) + package_dir = tmp_path / "apm_modules" / "repo" + package_dir.mkdir(parents=True) + outside = tmp_path / "outside.yml" + outside.write_text( + yaml.safe_dump( + { + "name": "evil", + "dependencies": {"lsp": [{"name": "evil", "command": "evil-lsp"}]}, + } + ), + encoding="utf-8", + ) + try: + (package_dir / "apm.yml").symlink_to(outside) + except OSError as exc: + pytest.skip(f"file symlinks unavailable: {exc}") + + assert scan_installed_executable_packages(manifest) == [] + + +def _write_cyclic_local_lockfile() -> None: + """Write a corrupt local dependency ancestry cycle.""" + from apm_cli.deps.lockfile import LockedDependency, LockFile + + lockfile = LockFile() + lockfile.add_dependency( + LockedDependency( + repo_url="_local/a", + source="local", + local_path="../a", + resolved_by="_local/b", + ) + ) + lockfile.add_dependency( + LockedDependency( + repo_url="_local/b", + source="local", + local_path="../b", + resolved_by="_local/a", + ) + ) + lockfile.write(Path("apm.lock.yaml")) + + def _isolated_config(tmp_path: Path): """Patch the user-config + legacy-approvals seams onto tmp_path.""" cfg = tmp_path / "config.json" @@ -331,6 +378,48 @@ def test_approve_unknown_package(self) -> None: assert result.exit_code == 0 assert "not found" in result.output + @pytest.mark.parametrize("command", [approve_cmd, deny_cmd]) + def test_trust_command_reports_corrupt_local_dependency_ancestry(self, command) -> None: + runner = CliRunner() + with runner.isolated_filesystem(): + _write_manifest(".") + Path("apm_modules").mkdir() + _write_cyclic_local_lockfile() + + result = runner.invoke(command, ["package-a"]) + + assert result.exit_code == 1 + assert "invalid local dependency ancestry" in result.output + assert "Run 'apm install'" in result.output + + def test_approve_uses_locked_identity_not_manifest_name(self) -> None: + runner = CliRunner() + with runner.isolated_filesystem(): + _write_manifest(".") + package_dir = Path("apm_modules/evil/repo") + _create_pkg_with_hooks(Path("apm_modules/evil"), "repo") + (package_dir / "apm.yml").write_text( + yaml.dump({"name": "trusted-name", "version": "1.0"}) + ) + from apm_cli.deps.lockfile import LockedDependency, LockFile + + locked = LockedDependency( + repo_url="evil/repo", + version="1.0", + name="trusted-name", + ) + lockfile = LockFile() + lockfile.add_dependency(locked) + lockfile.write(Path("apm.lock.yaml")) + + result = runner.invoke(approve_cmd, ["trusted-name"]) + + assert result.exit_code == 0, result.output + from apm_cli.utils.yaml_io import load_yaml + + stored = load_yaml(Path("apm.yml"))["executables"]["allow"] + assert set(stored) == {"evil/repo#1.0"} + def test_approve_recommended_bulk_accepts_org_set(self) -> None: runner = CliRunner() with runner.isolated_filesystem(): @@ -398,6 +487,12 @@ def test_deny_uninstalled_package(self) -> None: result = runner.invoke(deny_cmd, ["owner/repo"]) assert result.exit_code == 0 assert "Denied" in result.output + from apm_cli.security.executables import ALL_EXEC_TYPES + from apm_cli.utils.yaml_io import load_yaml + + stored = load_yaml(Path("apm.yml"))["executables"]["deny"]["owner/repo"] + assert set(stored) == set(ALL_EXEC_TYPES) + assert all(stored.values()) def test_deny_user_scope_writes_config(self, tmp_path: Path) -> None: p_cfg, p_legacy, cfg = _isolated_config(tmp_path) diff --git a/tests/unit/commands/test_install_context.py b/tests/unit/commands/test_install_context.py index 126f7262c..b68ab0ff5 100644 --- a/tests/unit/commands/test_install_context.py +++ b/tests/unit/commands/test_install_context.py @@ -77,6 +77,9 @@ class TestInstallContextFields: "target_decision", # issue #1620: per-invocation bin/ consent flag (--trust-bin/--no-trust-bin) "trust_bin", + # effective executable-trust projection consumed by service reconciliation + "exec_allow_map", + "exec_allow_resolved", ) def test_all_required_fields_present(self): diff --git a/tests/unit/commands/test_uninstall_atomic_selection.py b/tests/unit/commands/test_uninstall_atomic_selection.py index 48ead2b5a..19094814a 100644 --- a/tests/unit/commands/test_uninstall_atomic_selection.py +++ b/tests/unit/commands/test_uninstall_atomic_selection.py @@ -9,7 +9,7 @@ import pytest from click.testing import CliRunner -from apm_cli.commands.uninstall.cli import uninstall +from apm_cli.commands.uninstall.cli import _cleanup_stale_lsp, uninstall from apm_cli.commands.uninstall.engine import ( LocalSlotRefresh, _activate_staged_local_refresh, @@ -33,6 +33,29 @@ def _write_manifest(project: Path, dependencies: Sequence[object]) -> None: ) +def test_global_lsp_cleanup_failure_uses_global_recovery_command(tmp_path: Path) -> None: + """User-scope cleanup must not prescribe a project-scope reinstall.""" + logger = MagicMock() + with patch( + "apm_cli.install.lsp.integration.reconcile_lsp_after_uninstall", + side_effect=OSError("foreign config"), + ): + updated, error = _cleanup_stale_lsp( + apm_package=MagicMock(), + lockfile=MagicMock(), + lockfile_path=tmp_path / "apm.lock.yaml", + modules_dir=tmp_path / "apm_modules", + deploy_root=tmp_path, + user_scope=True, + logger=logger, + ) + + assert updated is False + assert isinstance(error, OSError) + message = logger.error.call_args.args[0] + assert "'apm install --global'" in message + + def test_uninstall_help_points_to_actionable_dependency_keys() -> None: """The terminal discovery surface names the inventory source.""" result = CliRunner().invoke(uninstall, ["--help"]) diff --git a/tests/unit/commands/test_update_command.py b/tests/unit/commands/test_update_command.py index 9413acf58..072986d8b 100644 --- a/tests/unit/commands/test_update_command.py +++ b/tests/unit/commands/test_update_command.py @@ -94,6 +94,7 @@ def test_service_only_required_failure_renders_before_exit(tmp_path: Path) -> No dry_run=False, logger=logger, verbose=False, + force=False, ) assert exc_info.value.code == 1 diff --git a/tests/unit/core/test_deployment_state.py b/tests/unit/core/test_deployment_state.py index dc8ae6608..4990a7b9f 100644 --- a/tests/unit/core/test_deployment_state.py +++ b/tests/unit/core/test_deployment_state.py @@ -659,6 +659,30 @@ def test_local_bundle_provenance_survives_canonical_ledger_rebuilds() -> None: assert DeploymentLedgerCodec.local_bundle_paths(rebuilt) == frozenset({renamed, sibling}) +def test_service_rows_with_same_name_and_runtime_keep_distinct_targets() -> None: + """MCP and LSP URI ownership must not collide in canonical state.""" + lockfile = LockFile() + DeploymentLedgerCodec.replace_mcp_target_servers( + lockfile, + {"claude": ["shared-server"]}, + ) + DeploymentLedgerCodec.replace_lsp_target_servers( + lockfile, + {"claude": ["shared-server"]}, + ) + + rebuilt = LockFile.from_yaml(lockfile.to_yaml()) + + assert rebuilt.mcp_target_servers == {"claude": ["shared-server"]} + assert rebuilt.lsp_target_servers == {"claude": ["shared-server"]} + service_targets = { + record.locator.target + for record in rebuilt.deployment_ledger.records.values() + if record.locator.value == "shared-server" + } + assert service_targets == {"mcp", "lsp"} + + def test_local_bundle_provenance_rejects_missing_or_malformed_hashes() -> None: bundled = ".agents/skills/bundled/SKILL.md" lockfile = LockFile() diff --git a/tests/unit/deps/test_path_anchoring.py b/tests/unit/deps/test_path_anchoring.py index bc05e46b7..a1b8518de 100644 --- a/tests/unit/deps/test_path_anchoring.py +++ b/tests/unit/deps/test_path_anchoring.py @@ -19,11 +19,16 @@ from __future__ import annotations from pathlib import Path +from unittest.mock import patch import pytest from apm_cli.deps.lockfile import LockedDependency, LockFile -from apm_cli.deps.path_anchoring import LocalResolutionError, resolve_local_dep_dir +from apm_cli.deps.path_anchoring import ( + LocalResolutionError, + resolve_local_dep_dir, + resolve_local_dep_dirs, +) def _local(repo_url: str, local_path: str, resolved_by: str | None = None) -> LockedDependency: @@ -53,6 +58,17 @@ def test_lockfile_none_ok_for_root_dep(self, tmp_path: Path) -> None: result = resolve_local_dep_dir(dep, None, tmp_path) assert result == (tmp_path / "packages" / "foo").resolve() + def test_root_dep_does_not_build_parent_index(self, tmp_path: Path) -> None: + """Wide direct-only graphs must retain constant-time singular resolution.""" + dep = _local("_local/foo", "./packages/foo") + with patch( + "apm_cli.deps.path_anchoring.build_local_parent_index", + side_effect=AssertionError("direct dependency built an index"), + ): + result = resolve_local_dep_dir(dep, _lockfile(dep), tmp_path) + + assert result == (tmp_path / "packages" / "foo").resolve() + class TestParentWalk: def test_single_hop(self, tmp_path: Path) -> None: @@ -68,6 +84,30 @@ def test_multi_hop_depth_three(self, tmp_path: Path) -> None: result = resolve_local_dep_dir(c, _lockfile(a, b, c), tmp_path) assert result == (tmp_path / "packages" / "c").resolve() + def test_shared_index_and_cache_resolve_chain_linearly(self, tmp_path: Path) -> None: + """Batch callers must resolve each ancestry edge at most once.""" + from apm_cli.deps import path_anchoring + + dependencies = [_local("_local/0", "./packages/0")] + for index in range(1, 40): + dependencies.append( + _local( + f"_local/{index}", + f"../{index}", + resolved_by=f"_local/{index - 1}", + ) + ) + lockfile = _lockfile(*dependencies) + with patch.object( + path_anchoring, + "_find_parent", + wraps=path_anchoring._find_parent, + ) as find_parent: + resolved = resolve_local_dep_dirs(lockfile, tmp_path) + + assert find_parent.call_count == len(dependencies) - 1 + assert len(resolved) == len(dependencies) + class TestAbsoluteBypass: def test_absolute_local_path_bypasses_anchor(self, tmp_path: Path) -> None: diff --git a/tests/unit/install/test_install_local_bundle.py b/tests/unit/install/test_install_local_bundle.py index d784e213b..86aac76da 100644 --- a/tests/unit/install/test_install_local_bundle.py +++ b/tests/unit/install/test_install_local_bundle.py @@ -142,6 +142,46 @@ def _invoke(project: Path, monkeypatch, *args: str): return _invoke_cli(project, monkeypatch, "install", *args) +def test_local_bundle_receives_enabled_empty_executable_gate( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Bundle LSP filtering must distinguish an enabled empty gate from disabled.""" + bundle = _make_bundle( + tmp_path / "source", + files={ + "lsp.json": json.dumps( + { + "lspServers": { + "unsafe": { + "command": "unsafe-language-server", + "extensionToLanguage": {".unsafe": "unsafe"}, + } + } + } + ) + }, + ) + project = _make_project(tmp_path / "destination") + manifest = yaml.safe_load((project / "apm.yml").read_text(encoding="utf-8")) + manifest["executables"] = {} + (project / "apm.yml").write_text(yaml.dump(manifest), encoding="utf-8") + captured: dict[str, object] = {} + + def _capture(**kwargs) -> None: + captured.update(kwargs) + + monkeypatch.setattr( + "apm_cli.install.local_bundle_handler.install_local_bundle", + _capture, + ) + + result = _invoke(project, monkeypatch, str(bundle), "--no-policy") + + assert result.exit_code == 0, result.output + assert captured["allow_executables"] == {} + + # --------------------------------------------------------------------------- # Duck-type contract test for package_info # --------------------------------------------------------------------------- @@ -804,6 +844,63 @@ def test_canvas_deployed_when_flag_on_no_enforcement(self, tmp_path, monkeypatch assert result.exit_code == 0, result.output assert (project / ".github" / "extensions" / "widget" / "extension.mjs").exists() + def test_canvas_requires_exact_bundle_content_approval(self, tmp_path, monkeypatch): + import apm_cli.config as _conf + from apm_cli.bundle.local_bundle import detect_local_bundle + from apm_cli.security.executables import local_bundle_approval_key + + monkeypatch.setattr(_conf, "_config_cache", {"experimental": {"canvas": True}}) + bundle = _make_bundle( + tmp_path / "source", + files={"extensions/widget/extension.mjs": "export default {};\n"}, + include_lockfile=False, + ) + bundle_info = detect_local_bundle(bundle) + assert bundle_info is not None + approval_key = local_bundle_approval_key( + bundle_info.package_id, + str(bundle_info.plugin_json.get("version") or ""), + bundle_info.source_dir, + bundle_info.lockfile, + ) + project = _make_project(tmp_path / "generic") + manifest_path = project / "apm.yml" + manifest = yaml.safe_load(manifest_path.read_text(encoding="utf-8")) + manifest["executables"] = {"allow": {"test-plugin": {"canvas": True}}} + manifest_path.write_text(yaml.safe_dump(manifest), encoding="utf-8") + + generic = _invoke(project, monkeypatch, str(bundle), "--target", "copilot") + + assert generic.exit_code == 0, generic.output + assert not (project / ".github" / "extensions" / "widget").exists() + assert approval_key in "".join(generic.output.split()) + assert "executables:" in generic.output + assert "allow:" in generic.output + assert "canvas: true" in generic.output + + manifest["executables"] = {"allow": {approval_key: {"canvas": True}}} + manifest_path.write_text(yaml.safe_dump(manifest), encoding="utf-8") + exact = _invoke(project, monkeypatch, str(bundle), "--target", "copilot") + assert exact.exit_code == 0, exact.output + assert (project / ".github" / "extensions" / "widget" / "extension.mjs").exists() + + (bundle / "extensions" / "widget" / "extension.mjs").write_text( + "export default { changed: true };\n", + encoding="utf-8", + ) + changed_project = _make_project(tmp_path / "changed") + changed_manifest = yaml.safe_load((changed_project / "apm.yml").read_text(encoding="utf-8")) + changed_manifest["executables"] = {"allow": {approval_key: {"canvas": True}}} + (changed_project / "apm.yml").write_text( + yaml.safe_dump(changed_manifest), + encoding="utf-8", + ) + + changed = _invoke(changed_project, monkeypatch, str(bundle), "--target", "copilot") + + assert changed.exit_code == 0, changed.output + assert not (changed_project / ".github" / "extensions" / "widget").exists() + def test_canvas_blocked_when_feature_off_regardless(self, tmp_path, monkeypatch): """When the experimental flag is off, canvas is always silently dropped.""" import apm_cli.config as _conf diff --git a/tests/unit/install/test_lsp_integration.py b/tests/unit/install/test_lsp_integration.py index 877b2033c..283dea5da 100644 --- a/tests/unit/install/test_lsp_integration.py +++ b/tests/unit/install/test_lsp_integration.py @@ -7,12 +7,20 @@ from __future__ import annotations +import json +from types import SimpleNamespace from unittest.mock import MagicMock, patch import pytest +from apm_cli.constants import InstallMode +from apm_cli.core.deployment_ledger import DeploymentLedgerCodec from apm_cli.deps.lockfile import LockFile -from apm_cli.install.lsp.integration import run_lsp_integration, run_owned_lsp_integration +from apm_cli.install.lsp.integration import ( + reconcile_lsp_after_uninstall, + run_lsp_integration, + run_owned_lsp_integration, +) from apm_cli.models.dependency.lsp import LSPDependency # --------------------------------------------------------------------------- @@ -36,10 +44,20 @@ def _mock_logger(): return logger -def _mock_lock(*, lsp_servers=None, lsp_configs=None): +def _mock_lock( + *, + lsp_servers=None, + lsp_configs=None, + lsp_config_provenance=None, + lsp_target_servers=None, + target_servers_present=False, +): lock = MagicMock() lock.lsp_servers = lsp_servers or [] lock.lsp_configs = lsp_configs or {} + lock.lsp_config_provenance = lsp_config_provenance or {} + lock.lsp_target_servers = lsp_target_servers or {} + lock._lsp_target_servers_present = target_servers_present return lock @@ -58,11 +76,14 @@ def test_records_owner_and_removes_only_its_stale_servers( lsp_servers=["old", "project"], lsp_configs={"old": {}, "project": {}}, lsp_config_provenance={"old": "bundle#1"}, + lsp_target_servers={"copilot": ["old"]}, + _lsp_target_servers_present=True, ).write(lock_path) dependency = _make_dep("new") mock_integrator.install.return_value = 1 mock_integrator.get_server_names.return_value = {"new"} mock_integrator.get_server_configs.return_value = {"new": dependency.to_lsp_json_entry()} + mock_integrator.supported_target_runtimes.return_value = ["copilot"] count = run_owned_lsp_integration( dependencies=[dependency], @@ -80,7 +101,8 @@ def test_records_owner_and_removes_only_its_stale_servers( lockfile = LockFile.read(lock_path) assert lockfile is not None assert lockfile.lsp_servers == ["new", "project"] - assert lockfile.lsp_config_provenance == {"new": "bundle#1"} + assert lockfile.lsp_config_provenance == {"new": "bundle:bundle#1"} + assert lockfile.lsp_target_servers == {"copilot": ["new"]} @patch(_PATCH_TARGET) def test_rejects_name_owned_by_another_source(self, mock_integrator, tmp_path) -> None: @@ -92,7 +114,7 @@ def test_rejects_name_owned_by_another_source(self, mock_integrator, tmp_path) - ).write(lock_path) mock_integrator.get_server_names.return_value = {"pyright"} - with pytest.raises(ValueError, match="conflicts with another owner"): + with pytest.raises(ValueError, match="conflicts with another owner") as exc_info: run_owned_lsp_integration( dependencies=[_make_dep("pyright")], owner="bundle#1", @@ -102,6 +124,86 @@ def test_rejects_name_owned_by_another_source(self, mock_integrator, tmp_path) - target_runtimes=["copilot"], logger=_mock_logger(), ) + assert "other#1" in str(exc_info.value) + assert "--force does not transfer ownership" in str(exc_info.value) + + @patch(_PATCH_TARGET) + def test_legacy_bundle_provenance_does_not_infer_current_target( + self, mock_integrator, tmp_path + ) -> None: + """A pre-target-map owner must not authorize deletion on a new target.""" + lock_path = tmp_path / "apm.lock.yaml" + LockFile( + lsp_servers=["shared"], + lsp_configs={"shared": {"command": "legacy"}}, + lsp_config_provenance={"shared": "bundle#1"}, + ).write(lock_path) + mock_integrator.get_server_names.return_value = set() + mock_integrator.get_server_configs.return_value = {} + + run_owned_lsp_integration( + dependencies=[], + owner="bundle#1", + lock_path=lock_path, + project_root=tmp_path, + user_scope=False, + target_runtimes=["claude"], + logger=_mock_logger(), + ) + + mock_integrator.remove_stale.assert_not_called() + + @patch(_PATCH_TARGET) + def test_bundle_id_cannot_impersonate_project_owner(self, mock_integrator, tmp_path) -> None: + lock_path = tmp_path / "apm.lock.yaml" + LockFile( + lsp_servers=["project"], + lsp_configs={"project": {"command": "project-lsp"}}, + lsp_config_provenance={"project": "project:."}, + lsp_target_servers={"claude": ["project"]}, + _lsp_target_servers_present=True, + ).write(lock_path) + mock_integrator.get_server_names.return_value = set() + + count = run_owned_lsp_integration( + dependencies=[], + owner="project:.", + lock_path=lock_path, + project_root=tmp_path, + user_scope=False, + target_runtimes=["claude"], + logger=_mock_logger(), + ) + + assert count == 0 + mock_integrator.remove_stale.assert_not_called() + lockfile = LockFile.read(lock_path) + assert lockfile is not None + assert lockfile.lsp_servers == ["project"] + assert lockfile.lsp_config_provenance == {"project": "project:."} + + def test_local_bundle_force_reaches_owned_lsp_writer(self, tmp_path) -> None: + from types import SimpleNamespace + + from apm_cli.install.local_bundle_handler import _wire_bundle_lsp_servers + + with patch( + "apm_cli.install.lsp.integration.run_owned_lsp_integration", + return_value=0, + ) as run_owned: + _wire_bundle_lsp_servers( + bundle_dir=tmp_path / "bundle", + targets=[SimpleNamespace(name="claude")], + project_root=tmp_path, + user_scope=False, + verbose=False, + logger=_mock_logger(), + deps=[_make_dep("pyright")], + owner="bundle#1", + force=True, + ) + + assert run_owned.call_args.kwargs["force"] is True # =========================================================================== @@ -129,6 +231,8 @@ def test_no_lsp_deps_no_old_servers(self, mock_integrator, tmp_path): assert count == 0 mock_integrator.install.assert_not_called() + mock_integrator.resolve_target_runtimes.assert_not_called() + mock_integrator.update_lockfile.assert_not_called() @patch(_PATCH_TARGET) def test_installs_direct_deps(self, mock_integrator, tmp_path): @@ -159,13 +263,54 @@ def test_installs_direct_deps(self, mock_integrator, tmp_path): assert count == 1 mock_integrator.install.assert_called_once() + @patch(_PATCH_TARGET) + def test_manifest_conflict_names_bundle_owner_and_recovery(self, mock_integrator, tmp_path): + """A cross-owner conflict must not imply that --force transfers it.""" + dependency = _make_dep("shared") + package = MagicMock() + package.get_lsp_dependencies.return_value = [dependency] + old_lock = _mock_lock( + lsp_servers=["shared"], + lsp_configs={"shared": {}}, + lsp_config_provenance={"shared": "bundle:vendor/tool"}, + lsp_target_servers={"claude": ["shared"]}, + target_servers_present=True, + ) + mock_integrator.resolve_target_runtimes.return_value = ["claude"] + mock_integrator.get_server_names.return_value = {"shared"} + + with pytest.raises( + ValueError, + match="installed bundle owner", + ) as exc_info: + run_lsp_integration( + apm_package=package, + apm_modules_path=tmp_path / "apm_modules", + lock_path=tmp_path / "apm.lock.yaml", + existing_lock=old_lock, + project_root=tmp_path, + user_scope=False, + should_install=True, + logger=_mock_logger(), + ) + + assert "bundle:vendor/tool" in str(exc_info.value) + assert "--force does not transfer ownership" in str(exc_info.value) + @patch(_PATCH_TARGET) def test_filters_unapproved_lsp_dependencies(self, mock_integrator, tmp_path): """An explicit executable gate blocks LSP servers until approved.""" apm_package = MagicMock() - apm_package.get_lsp_dependencies.return_value = [_make_dep("pyright")] + apm_package.get_lsp_dependencies.return_value = [] apm_package.allow_executables = {} - mock_integrator.collect_transitive.return_value = [] + mock_integrator.collect_transitive.return_value = [ + _make_dep( + "pyright", + resolved_by="owner/package", + approval_keys=("owner/package",), + ) + ] + mock_integrator.deduplicate.return_value = [] mock_integrator.get_server_names.return_value = set() mock_integrator.get_server_configs.return_value = {} modules = tmp_path / "apm_modules" @@ -180,6 +325,7 @@ def test_filters_unapproved_lsp_dependencies(self, mock_integrator, tmp_path): user_scope=False, should_install=True, logger=_mock_logger(), + effective_allow_executables={}, ) assert count == 0 @@ -220,6 +366,8 @@ def test_resolves_targets_for_install(self, mock_integrator, tmp_path): diagnostics=None, target_runtimes=["copilot"], fail_on_write_error=False, + managed_target_servers={}, + force=False, ) @patch(_PATCH_TARGET) @@ -254,6 +402,227 @@ def test_deduplicates_transitive(self, mock_integrator, tmp_path): assert count == 2 mock_integrator.deduplicate.assert_called_once() + @patch(_PATCH_TARGET) + def test_precomputed_disabled_allow_map_does_not_rediscover_policy( + self, mock_integrator, tmp_path + ): + """A resolved disabled gate is distinct from an unresolved trust map.""" + apm_package = MagicMock() + apm_package.get_lsp_dependencies.return_value = [] + transitive = _make_dep( + "pyright", + resolved_by="owner/package", + approval_keys=("owner/package",), + ) + mock_integrator.collect_transitive.return_value = [transitive] + mock_integrator.deduplicate.return_value = [transitive] + mock_integrator.resolve_target_runtimes.return_value = ["claude"] + mock_integrator.install.return_value = 1 + mock_integrator.get_server_names.return_value = {"pyright"} + mock_integrator.get_server_configs.return_value = {"pyright": {}} + modules = tmp_path / "apm_modules" + modules.mkdir() + + with patch("apm_cli.policy.discovery.discover_policy_with_chain") as discover: + count = run_lsp_integration( + apm_package=apm_package, + apm_modules_path=modules, + lock_path=tmp_path / "apm.lock.yaml", + existing_lock=None, + project_root=tmp_path, + user_scope=False, + should_install=True, + logger=_mock_logger(), + effective_allow_executables=None, + effective_allow_resolved=True, + ) + + assert count == 1 + discover.assert_not_called() + + +def test_mcp_only_service_install_does_not_reconcile_lsp(tmp_path) -> None: + """The explicit MCP filter must not deploy an LSP plugin.""" + from apm_cli.core.scope import InstallScope + from apm_cli.install.service_integration import run_service_integrations + + context = SimpleNamespace( + project_root=tmp_path, + scope=InstallScope.PROJECT, + install_mode=InstallMode.MCP, + logger=_mock_logger(), + runtime=None, + exclude=None, + trust_transitive_mcp=False, + no_policy=True, + verbose=False, + force=False, + exec_allow_map=None, + exec_allow_resolved=True, + ) + package = MagicMock() + package.get_lsp_dependencies.return_value = [_make_dep("must-not-deploy")] + target_decision = MagicMock() + target_decision.value = ["claude"] + with ( + patch("apm_cli.install.mcp.run_mcp_integration", return_value=(0, {})), + patch("apm_cli.install.lsp.run_lsp_integration", return_value=0) as run_lsp, + ): + run_service_integrations( + context, + apm_package=package, + mcp_deps=[], + lock_path=tmp_path / "apm.lock.yaml", + existing_lock=None, + old_mcp_servers=set(), + old_mcp_configs={}, + old_mcp_provenance={}, + old_mcp_target_servers={}, + old_mcp_target_servers_present=True, + diagnostics=None, + explicit_target=["claude"], + target_decision=target_decision, + ) + + assert run_lsp.call_args.kwargs["should_install"] is False + + +def test_legacy_lock_name_does_not_authorize_foreign_plugin_overwrite(tmp_path) -> None: + """Legacy `.lsp.json` state proves no ownership of the new plugin path.""" + plugin_path = tmp_path / ".claude" / "skills" / "apm-lsp" / ".claude-plugin" / "plugin.json" + plugin_path.parent.mkdir(parents=True) + foreign = b'{"name":"apm-lsp","lspServers":{"pyright":{"command":"foreign"}}}\n' + plugin_path.write_bytes(foreign) + lock_path = tmp_path / "apm.lock.yaml" + old_lock = LockFile( + lsp_servers=["pyright"], + lsp_configs={"pyright": {"command": "legacy"}}, + ) + old_lock.write(lock_path) + package = MagicMock() + package.get_lsp_dependencies.return_value = [_make_dep("pyright")] + from apm_cli.install.errors import RequiredIntegrationError + + with pytest.raises(RequiredIntegrationError, match="not managed by APM"): + run_lsp_integration( + apm_package=package, + apm_modules_path=tmp_path / "apm_modules", + lock_path=lock_path, + existing_lock=old_lock, + project_root=tmp_path, + user_scope=False, + should_install=True, + logger=_mock_logger(), + runtime="claude", + fail_on_write_error=True, + ) + + assert plugin_path.read_bytes() == foreign + + +def test_uninstall_reconciliation_removes_only_departed_package_lsp(tmp_path) -> None: + """Uninstall must revoke one package without touching surviving owners.""" + plugin_path = tmp_path / ".claude" / "skills" / "apm-lsp" / ".claude-plugin" / "plugin.json" + plugin_path.parent.mkdir(parents=True) + plugin_path.write_text( + '{"name":"apm-lsp","lspServers":' + '{"removed":{"command":"gone"},"root":{"command":"keep"},' + '"bundle":{"command":"keep"}}}\n', + encoding="ascii", + ) + configs = { + name: {"name": name, "command": command} + for name, command in ( + ("removed", "gone"), + ("root", "keep"), + ("bundle", "keep"), + ) + } + lockfile = LockFile( + lsp_servers=list(configs), + lsp_configs=configs, + lsp_config_provenance={ + "removed": "package:departed/package", + "root": "project:.", + "bundle": "bundle:local-bundle", + }, + ) + DeploymentLedgerCodec.replace_lsp_target_servers( + lockfile, + {"claude": list(configs)}, + ) + package = MagicMock() + package.get_lsp_dependencies.return_value = [_make_dep("root")] + + changed = reconcile_lsp_after_uninstall( + apm_package=package, + lockfile=lockfile, + lock_path=tmp_path / "apm.lock.yaml", + modules_dir=tmp_path / "apm_modules", + project_root=tmp_path, + user_scope=False, + logger=_mock_logger(), + ) + + assert changed is True + assert set(lockfile.lsp_servers) == {"root", "bundle"} + assert lockfile.lsp_target_servers == {"claude": ["bundle", "root"]} + assert set(json.loads(plugin_path.read_text())["lspServers"]) == {"root", "bundle"} + + +def test_uninstall_transfers_first_wins_lsp_to_surviving_package(tmp_path) -> None: + """A same-name survivor must replace the departed declaration.""" + from apm_cli.deps.lockfile import LockedDependency + from apm_cli.models.apm_package import APMPackage + + manifest = tmp_path / "apm.yml" + manifest.write_text( + "name: root\nversion: 1.0.0\ntargets: [claude]\n", + encoding="ascii", + ) + plugin_path = tmp_path / ".claude" / "skills" / "apm-lsp" / ".claude-plugin" / "plugin.json" + plugin_path.parent.mkdir(parents=True) + plugin_path.write_text( + '{"name":"apm-lsp","lspServers":{"shared":{"command":"departed"}}}\n', + encoding="ascii", + ) + lockfile = LockFile( + lsp_servers=["shared"], + lsp_configs={"shared": {"name": "shared", "command": "departed"}}, + lsp_config_provenance={"shared": "package:departed/package"}, + ) + lockfile.add_dependency(LockedDependency(repo_url="surviving/package")) + DeploymentLedgerCodec.replace_lsp_target_servers( + lockfile, + {"claude": ["shared"]}, + ) + survivor = _make_dep( + "shared", + command="survivor", + resolved_by="surviving/package", + approval_keys=("surviving/package",), + ) + modules_dir = tmp_path / "apm_modules" + modules_dir.mkdir() + + with patch( + "apm_cli.integration.lsp_integrator.LSPIntegrator.collect_transitive", + return_value=[survivor], + ): + changed = reconcile_lsp_after_uninstall( + apm_package=APMPackage.from_apm_yml(manifest), + lockfile=lockfile, + lock_path=tmp_path / "apm.lock.yaml", + modules_dir=modules_dir, + project_root=tmp_path, + user_scope=False, + logger=_mock_logger(), + ) + + assert changed is True + assert lockfile.lsp_config_provenance == {"shared": "package:surviving/package"} + assert json.loads(plugin_path.read_text())["lspServers"]["shared"]["command"] == "survivor" + # =========================================================================== # Stale cleanup @@ -267,8 +636,17 @@ def test_removes_stale_servers(self, mock_integrator, tmp_path): apm_package = MagicMock() apm_package.get_lsp_dependencies.return_value = [_make_dep("pyright")] - old_lock = _mock_lock(lsp_servers=["pyright", "old-server"]) + old_lock = _mock_lock( + lsp_servers=["pyright", "old-server"], + lsp_config_provenance={ + "pyright": "project:.", + "old-server": "project:.", + }, + lsp_target_servers={"claude": ["pyright", "old-server"]}, + target_servers_present=True, + ) + mock_integrator.resolve_target_runtimes.return_value = ["claude"] mock_integrator.collect_transitive.return_value = [] mock_integrator.install.return_value = 1 mock_integrator.get_server_names.return_value = {"pyright"} @@ -298,7 +676,16 @@ def test_removes_all_old_when_no_deps_remain(self, mock_integrator, tmp_path): apm_package = MagicMock() apm_package.get_lsp_dependencies.return_value = [] - old_lock = _mock_lock(lsp_servers=["old-a", "old-b"]) + old_lock = _mock_lock( + lsp_servers=["old-a", "old-b"], + lsp_config_provenance={ + "old-a": "project:.", + "old-b": "project:.", + }, + lsp_target_servers={"claude": ["old-a", "old-b"]}, + target_servers_present=True, + ) + mock_integrator.resolve_target_runtimes.return_value = ["claude"] run_lsp_integration( apm_package=apm_package, @@ -346,3 +733,77 @@ def test_restores_old_lockfile_when_not_installing(self, mock_integrator, tmp_pa mock_integrator.update_lockfile.assert_called_once() mock_integrator.install.assert_not_called() + + @patch(_PATCH_TARGET) + def test_legacy_lock_names_do_not_authorize_new_plugin_overwrite( + self, mock_integrator, tmp_path + ): + """A pre-plugin lock name is not path-level ownership.""" + dependency = _make_dep("pyright") + apm_package = MagicMock() + apm_package.get_lsp_dependencies.return_value = [dependency] + old_lock = _mock_lock( + lsp_servers=["pyright"], + lsp_configs={"pyright": {"command": "legacy"}}, + ) + mock_integrator.resolve_target_runtimes.return_value = ["claude"] + mock_integrator.collect_transitive.return_value = [] + mock_integrator.install.return_value = 1 + mock_integrator.get_server_names.return_value = {"pyright"} + mock_integrator.get_server_configs.return_value = {"pyright": {}} + + run_lsp_integration( + apm_package=apm_package, + apm_modules_path=tmp_path / "apm_modules", + lock_path=tmp_path / "apm.lock.yaml", + existing_lock=old_lock, + project_root=tmp_path, + user_scope=False, + should_install=True, + logger=_mock_logger(), + ) + + assert mock_integrator.install.call_args.kwargs["managed_target_servers"] == {} + + @patch(_PATCH_TARGET) + def test_target_contraction_removes_only_recorded_old_target(self, mock_integrator, tmp_path): + """Switching from Claude to Copilot revokes the old Claude entry.""" + dependency = _make_dep("pyright") + apm_package = MagicMock() + apm_package.get_lsp_dependencies.return_value = [dependency] + old_lock = _mock_lock( + lsp_servers=["pyright"], + lsp_configs={"pyright": {}}, + lsp_config_provenance={"pyright": "project:."}, + lsp_target_servers={"claude": ["pyright"]}, + target_servers_present=True, + ) + mock_integrator.resolve_target_runtimes.return_value = ["copilot"] + mock_integrator.collect_transitive.return_value = [] + mock_integrator.install.return_value = 1 + mock_integrator.get_server_names.return_value = {"pyright"} + mock_integrator.get_server_configs.return_value = {"pyright": {}} + + run_lsp_integration( + apm_package=apm_package, + apm_modules_path=tmp_path / "apm_modules", + lock_path=tmp_path / "apm.lock.yaml", + existing_lock=old_lock, + project_root=tmp_path, + user_scope=False, + should_install=True, + logger=_mock_logger(), + ) + + mock_integrator.remove_stale.assert_called_once_with( + {"pyright"}, + project_root=tmp_path, + user_scope=False, + logger=mock_integrator.install.call_args.kwargs["logger"], + target_runtimes=["claude"], + fail_on_write_error=False, + ) + assert mock_integrator.update_lockfile.call_args.kwargs["lsp_target_servers"] == { + "claude": set(), + "copilot": {"pyright"}, + } diff --git a/tests/unit/integration/test_lsp_integrator.py b/tests/unit/integration/test_lsp_integrator.py index 868ecf0e0..e82d3af22 100644 --- a/tests/unit/integration/test_lsp_integrator.py +++ b/tests/unit/integration/test_lsp_integrator.py @@ -9,13 +9,15 @@ import json from pathlib import Path -from unittest.mock import patch +from unittest.mock import MagicMock, patch import pytest from apm_cli.integration.lsp_integrator import LSPIntegrator from apm_cli.models.dependency.lsp import LSPDependency +_CLAUDE_PROJECT_PLUGIN = Path(".claude") / "skills" / "apm-lsp" / ".claude-plugin" / "plugin.json" + # --------------------------------------------------------------------------- # Helpers # --------------------------------------------------------------------------- @@ -139,35 +141,155 @@ def test_empty_deps_returns_zero(self, tmp_path): count = LSPIntegrator.install([], project_root=tmp_path) assert count == 0 - def test_creates_lsp_json(self, tmp_path): + def test_creates_claude_plugin_manifest(self, tmp_path): deps = [_make_dep("pyright")] count = LSPIntegrator.install(deps, project_root=tmp_path) assert count == 1 - lsp_json = tmp_path / ".lsp.json" - assert lsp_json.exists() - data = json.loads(lsp_json.read_text()) - assert "pyright" in data - assert data["pyright"]["command"] == "pyright-langserver" - assert "name" not in data["pyright"] # name is the key, not in value + plugin_json = tmp_path / _CLAUDE_PROJECT_PLUGIN + assert plugin_json.exists() + data = json.loads(plugin_json.read_text()) + assert data["name"] == "apm-lsp" + assert data["lspServers"]["pyright"]["command"] == "pyright-langserver" + assert "name" not in data["lspServers"]["pyright"] + + def test_claude_plugin_install_prompts_runtime_reload(self, tmp_path): + logger = MagicMock() + + LSPIntegrator.install([_make_dep("pyright")], project_root=tmp_path, logger=logger) - def test_merges_with_existing_lsp_json(self, tmp_path): - lsp_json = tmp_path / ".lsp.json" - lsp_json.write_text(json.dumps({"existing-server": {"command": "x"}})) + logger.progress.assert_any_call( + " |-- run /reload-plugins or restart Claude Code to activate" + ) + + def test_refuses_existing_foreign_claude_plugin_manifest(self, tmp_path): + plugin_json = tmp_path / _CLAUDE_PROJECT_PLUGIN + plugin_json.parent.mkdir(parents=True) + plugin_json.write_text( + json.dumps( + { + "name": "custom-lsp-plugin", + "description": "Preserve this metadata", + "lspServers": {"existing-server": {"command": "x"}}, + } + ) + ) deps = [_make_dep("new-server")] - LSPIntegrator.install(deps, project_root=tmp_path) + from apm_cli.install.errors import RequiredIntegrationError + + with pytest.raises(RequiredIntegrationError, match="not APM"): + LSPIntegrator.install( + deps, + project_root=tmp_path, + fail_on_write_error=True, + ) + + assert json.loads(plugin_json.read_text())["name"] == "custom-lsp-plugin" + + def test_refuses_non_object_plugin_without_overwriting(self, tmp_path): + plugin_json = tmp_path / _CLAUDE_PROJECT_PLUGIN + plugin_json.parent.mkdir(parents=True) + original = b"[]\n" + plugin_json.write_bytes(original) + from apm_cli.install.errors import RequiredIntegrationError + + with pytest.raises(RequiredIntegrationError, match="must contain a JSON object"): + LSPIntegrator.install( + [_make_dep("pyright")], + project_root=tmp_path, + fail_on_write_error=True, + ) + + assert plugin_json.read_bytes() == original + + def test_refuses_unnamed_plugin_without_adopting_it(self, tmp_path): + plugin_json = tmp_path / _CLAUDE_PROJECT_PLUGIN + plugin_json.parent.mkdir(parents=True) + original = b'{"lspServers":{"user-owned":{"command":"keep"}}}\n' + plugin_json.write_bytes(original) + from apm_cli.install.errors import RequiredIntegrationError + + with pytest.raises(RequiredIntegrationError, match="unnamed"): + LSPIntegrator.install( + [_make_dep("pyright")], + project_root=tmp_path, + fail_on_write_error=True, + ) + + assert plugin_json.read_bytes() == original + + def test_force_merges_with_existing_foreign_claude_plugin_manifest(self, tmp_path): + plugin_json = tmp_path / _CLAUDE_PROJECT_PLUGIN + plugin_json.parent.mkdir(parents=True) + plugin_json.write_text( + json.dumps( + { + "name": "custom-lsp-plugin", + "description": "Preserve this metadata", + "lspServers": {"existing-server": {"command": "x"}}, + } + ) + ) + + LSPIntegrator.install([_make_dep("new-server")], project_root=tmp_path, force=True) + + data = json.loads(plugin_json.read_text()) + assert data["name"] == "apm-lsp" + assert data["description"] == "Preserve this metadata" + assert set(data["lspServers"]) == {"existing-server", "new-server"} + assert ( + LSPIntegrator.install( + [_make_dep("new-server")], + project_root=tmp_path, + managed_server_names={"new-server"}, + fail_on_write_error=True, + ) + == 0 + ) + LSPIntegrator.remove_stale( + {"new-server"}, + project_root=tmp_path, + fail_on_write_error=True, + ) + assert set(json.loads(plugin_json.read_text())["lspServers"]) == {"existing-server"} - data = json.loads(lsp_json.read_text()) - assert "existing-server" in data - assert "new-server" in data + def test_multitarget_collision_is_validated_before_any_write(self, tmp_path): + from apm_cli.install.errors import RequiredIntegrationError + + plugin_json = tmp_path / _CLAUDE_PROJECT_PLUGIN + plugin_json.parent.mkdir(parents=True) + plugin_json.write_text(json.dumps({"name": "foreign-plugin", "lspServers": {}})) + + with pytest.raises(RequiredIntegrationError, match="target 'claude'"): + LSPIntegrator.install( + [_make_dep("new-server")], + project_root=tmp_path, + target_runtimes=["copilot", "claude"], + fail_on_write_error=True, + ) + + assert not (tmp_path / ".github" / "lsp.json").exists() + assert json.loads(plugin_json.read_text())["name"] == "foreign-plugin" def test_update_existing_server_counts_as_change(self, tmp_path): - lsp_json = tmp_path / ".lsp.json" - lsp_json.write_text(json.dumps({"pyright": {"command": "old-cmd"}})) + plugin_json = tmp_path / _CLAUDE_PROJECT_PLUGIN + plugin_json.parent.mkdir(parents=True) + plugin_json.write_text( + json.dumps( + { + "name": "apm-lsp", + "lspServers": {"pyright": {"command": "old-cmd"}}, + } + ) + ) deps = [_make_dep("pyright")] - count = LSPIntegrator.install(deps, project_root=tmp_path) + count = LSPIntegrator.install( + deps, + project_root=tmp_path, + managed_server_names={"pyright"}, + ) assert count == 1 # changed config counts def test_no_change_returns_zero(self, tmp_path): @@ -182,14 +304,110 @@ def test_dict_deps_handled(self, tmp_path): deps = [{"name": "dict-server", "command": "x", "extensionToLanguage": {".py": "python"}}] count = LSPIntegrator.install(deps, project_root=tmp_path) assert count == 1 - data = json.loads((tmp_path / ".lsp.json").read_text()) - assert "dict-server" in data + data = json.loads((tmp_path / _CLAUDE_PROJECT_PLUGIN).read_text()) + assert "dict-server" in data["lspServers"] def test_multiple_servers(self, tmp_path): deps = [_make_dep("pyright"), _make_dep("ruff-lsp")] count = LSPIntegrator.install(deps, project_root=tmp_path) assert count == 2 + def test_rejects_symlink_escape_before_writing_claude_plugin(self, tmp_path): + project = tmp_path / "project" + outside = tmp_path / "outside" + project.mkdir() + outside.mkdir() + try: + (project / ".claude").symlink_to(outside, target_is_directory=True) + except OSError as exc: + pytest.skip(f"directory symlinks unavailable: {exc}") + + from apm_cli.install.errors import RequiredIntegrationError + + with pytest.raises(RequiredIntegrationError, match="symlinked path component"): + LSPIntegrator.install( + [_make_dep("pyright")], + project_root=project, + fail_on_write_error=True, + ) + + assert not (outside / "skills" / "apm-lsp" / ".claude-plugin" / "plugin.json").exists() + + def test_refuses_foreign_server_collision_without_force(self, tmp_path): + plugin_json = tmp_path / _CLAUDE_PROJECT_PLUGIN + plugin_json.parent.mkdir(parents=True) + plugin_json.write_text( + json.dumps( + { + "name": "apm-lsp", + "lspServers": {"pyright": {"command": "user-command"}}, + } + ) + ) + from apm_cli.install.errors import RequiredIntegrationError + + with pytest.raises(RequiredIntegrationError, match="not managed by APM"): + LSPIntegrator.install( + [_make_dep("pyright")], + project_root=tmp_path, + fail_on_write_error=True, + ) + + assert json.loads(plugin_json.read_text())["lspServers"]["pyright"] == { + "command": "user-command" + } + + def test_other_target_ownership_does_not_authorize_claude_overwrite(self, tmp_path): + plugin_json = tmp_path / _CLAUDE_PROJECT_PLUGIN + plugin_json.parent.mkdir(parents=True) + plugin_json.write_text( + json.dumps( + { + "name": "apm-lsp", + "lspServers": {"pyright": {"command": "user-command"}}, + } + ) + ) + from apm_cli.install.errors import RequiredIntegrationError + + with pytest.raises(RequiredIntegrationError, match="not managed by APM"): + LSPIntegrator.install( + [_make_dep("pyright")], + project_root=tmp_path, + target_runtimes=["claude"], + managed_target_servers={"copilot": {"pyright"}}, + fail_on_write_error=True, + ) + + assert json.loads(plugin_json.read_text())["lspServers"]["pyright"] == { + "command": "user-command" + } + + def test_managed_server_does_not_adopt_foreign_skill_content(self, tmp_path): + plugin_json = tmp_path / _CLAUDE_PROJECT_PLUGIN + plugin_json.parent.mkdir(parents=True) + plugin_json.write_text( + json.dumps( + { + "name": "apm-lsp", + "lspServers": {"pyright": {"command": "old-command"}}, + } + ) + ) + skill_root = plugin_json.parent.parent + (skill_root / "SKILL.md").write_text("# user-owned\n", encoding="utf-8") + from apm_cli.install.errors import RequiredIntegrationError + + with pytest.raises(RequiredIntegrationError, match="unowned content"): + LSPIntegrator.install( + [_make_dep("pyright")], + project_root=tmp_path, + managed_server_names={"pyright"}, + fail_on_write_error=True, + ) + + assert (skill_root / "SKILL.md").is_file() + # =========================================================================== # LSPIntegrator.install -- user scope @@ -197,28 +415,65 @@ def test_multiple_servers(self, tmp_path): class TestInstallUserScope: - def test_writes_to_claude_json(self, tmp_path): - claude_json = tmp_path / ".claude.json" + def test_writes_to_personal_claude_plugin_manifest(self, tmp_path): + plugin_json = tmp_path / _CLAUDE_PROJECT_PLUGIN with patch("apm_cli.integration.lsp_integrator.Path.home", return_value=tmp_path): deps = [_make_dep("pyright")] count = LSPIntegrator.install(deps, user_scope=True) assert count == 1 - data = json.loads(claude_json.read_text()) - assert "lspServers" in data + assert not (tmp_path / ".claude.json").exists() + data = json.loads(plugin_json.read_text()) + assert data["name"] == "apm-lsp" assert "pyright" in data["lspServers"] + assert "enabledPlugins" not in data - def test_merges_with_existing_claude_json(self, tmp_path): - claude_json = tmp_path / ".claude.json" - claude_json.write_text(json.dumps({"existingKey": True})) + def test_user_scope_reinstall_is_idempotent(self, tmp_path): + plugin_json = tmp_path / _CLAUDE_PROJECT_PLUGIN + dep = _make_dep("pyright") + + with patch("apm_cli.integration.lsp_integrator.Path.home", return_value=tmp_path): + assert LSPIntegrator.install([dep], user_scope=True) == 1 + before = plugin_json.read_text(encoding="utf-8") + assert LSPIntegrator.install([dep], user_scope=True) == 0 + + assert plugin_json.read_text(encoding="utf-8") == before + + def test_merges_with_existing_personal_claude_plugin_manifest(self, tmp_path): + plugin_json = tmp_path / _CLAUDE_PROJECT_PLUGIN + plugin_json.parent.mkdir(parents=True) + plugin_json.write_text(json.dumps({"name": "apm-lsp", "description": "keep"})) with patch("apm_cli.integration.lsp_integrator.Path.home", return_value=tmp_path): LSPIntegrator.install([_make_dep("ruff")], user_scope=True) - data = json.loads(claude_json.read_text()) - assert data["existingKey"] is True + data = json.loads(plugin_json.read_text()) + assert data["name"] == "apm-lsp" + assert data["description"] == "keep" assert "ruff" in data["lspServers"] + def test_rejects_symlinked_user_plugin_path_before_writing(self, tmp_path): + outside = tmp_path / "outside" + outside.mkdir() + claude_dir = tmp_path / ".claude" + try: + claude_dir.symlink_to(outside, target_is_directory=True) + except OSError as exc: + pytest.skip(f"directory symlinks unavailable: {exc}") + from apm_cli.install.errors import RequiredIntegrationError + + with ( + patch("apm_cli.integration.lsp_integrator.Path.home", return_value=tmp_path), + pytest.raises(RequiredIntegrationError, match="symlinked path component"), + ): + LSPIntegrator.install( + [_make_dep("pyright")], + user_scope=True, + fail_on_write_error=True, + ) + + assert not (outside / "skills" / "apm-lsp" / ".claude-plugin" / "plugin.json").exists() + class TestInstallCopilotTarget: def test_writes_project_lsp_json_with_file_extensions(self, tmp_path): @@ -291,38 +546,78 @@ def test_targets_copilot_when_binary_present(self, tmp_path): class TestRemoveStale: def test_empty_stale_set_is_noop(self, tmp_path): - lsp_json = tmp_path / ".lsp.json" - lsp_json.write_text(json.dumps({"keep": {"command": "x"}})) + plugin_json = tmp_path / _CLAUDE_PROJECT_PLUGIN + plugin_json.parent.mkdir(parents=True) + plugin_json.write_text( + json.dumps({"name": "apm-lsp", "lspServers": {"keep": {"command": "x"}}}) + ) LSPIntegrator.remove_stale(set(), project_root=tmp_path) - data = json.loads(lsp_json.read_text()) - assert "keep" in data + data = json.loads(plugin_json.read_text()) + assert "keep" in data["lspServers"] - def test_removes_stale_from_project_lsp_json(self, tmp_path): - lsp_json = tmp_path / ".lsp.json" - lsp_json.write_text( + def test_removes_stale_from_project_plugin_manifest(self, tmp_path): + plugin_json = tmp_path / _CLAUDE_PROJECT_PLUGIN + plugin_json.parent.mkdir(parents=True) + plugin_json.write_text( json.dumps( { - "keep": {"command": "x"}, - "stale": {"command": "y"}, + "name": "apm-lsp", + "lspServers": { + "keep": {"command": "x"}, + "stale": {"command": "y"}, + }, } ) ) + logger = MagicMock() + LSPIntegrator.remove_stale({"stale"}, project_root=tmp_path, logger=logger) + data = json.loads(plugin_json.read_text()) + assert "keep" in data["lspServers"] + assert "stale" not in data["lspServers"] + logger.progress.assert_any_call( + " |-- run /reload-plugins or restart Claude Code to activate" + ) + + def test_removing_last_server_deletes_owned_project_plugin(self, tmp_path): + plugin_json = tmp_path / _CLAUDE_PROJECT_PLUGIN + plugin_json.parent.mkdir(parents=True) + plugin_json.write_text( + json.dumps({"name": "apm-lsp", "lspServers": {"stale": {"command": "y"}}}) + ) + LSPIntegrator.remove_stale({"stale"}, project_root=tmp_path) - data = json.loads(lsp_json.read_text()) - assert "keep" in data - assert "stale" not in data - def test_removes_stale_from_user_claude_json(self, tmp_path): - claude_json = tmp_path / ".claude.json" - claude_json.write_text( + assert not plugin_json.exists() + assert not plugin_json.parent.parent.exists() + + def test_removing_last_user_claude_server_removes_plugin_directory(self, tmp_path): + plugin_json = tmp_path / _CLAUDE_PROJECT_PLUGIN + plugin_json.parent.mkdir(parents=True) + plugin_json.write_text( + json.dumps({"name": "apm-lsp", "lspServers": {"stale": {"command": "y"}}}), + encoding="utf-8", + ) + + with patch("apm_cli.integration.lsp_integrator.Path.home", return_value=tmp_path): + LSPIntegrator.remove_stale({"stale"}, user_scope=True) + + assert tmp_path.exists() + assert not plugin_json.exists() + assert not plugin_json.parent.parent.exists() + + def test_removes_stale_from_user_claude_plugin_manifest(self, tmp_path): + plugin_json = tmp_path / _CLAUDE_PROJECT_PLUGIN + plugin_json.parent.mkdir(parents=True) + plugin_json.write_text( json.dumps( { + "name": "apm-lsp", "lspServers": { "keep": {"command": "x"}, "stale": {"command": "y"}, - } + }, } ) ) @@ -330,18 +625,19 @@ def test_removes_stale_from_user_claude_json(self, tmp_path): with patch("apm_cli.integration.lsp_integrator.Path.home", return_value=tmp_path): LSPIntegrator.remove_stale({"stale"}, user_scope=True) - data = json.loads(claude_json.read_text()) + data = json.loads(plugin_json.read_text()) + assert data["name"] == "apm-lsp" assert "keep" in data["lspServers"] assert "stale" not in data["lspServers"] - def test_no_lsp_json_is_noop(self, tmp_path): - # Should not raise even if .lsp.json does not exist + def test_no_plugin_manifest_is_noop(self, tmp_path): LSPIntegrator.remove_stale({"nonexistent"}, project_root=tmp_path) def test_strict_cleanup_raises_on_unwritable_config_shape(self, tmp_path): from apm_cli.install.errors import RequiredIntegrationError - (tmp_path / ".lsp.json").mkdir() + config_path = tmp_path / _CLAUDE_PROJECT_PLUGIN + config_path.mkdir(parents=True) with pytest.raises(RequiredIntegrationError, match="LSP cleanup failed"): LSPIntegrator.remove_stale( @@ -350,21 +646,49 @@ def test_strict_cleanup_raises_on_unwritable_config_shape(self, tmp_path): fail_on_write_error=True, ) + def test_cleanup_refuses_foreign_plugin_manifest(self, tmp_path): + from apm_cli.install.errors import RequiredIntegrationError + + plugin_json = tmp_path / _CLAUDE_PROJECT_PLUGIN + plugin_json.parent.mkdir(parents=True) + plugin_json.write_text( + json.dumps( + { + "name": "user-plugin", + "lspServers": {"stale": {"command": "keep"}}, + } + ), + encoding="utf-8", + ) + + with pytest.raises(RequiredIntegrationError, match="not owned by APM"): + LSPIntegrator.remove_stale( + {"stale"}, + project_root=tmp_path, + fail_on_write_error=True, + ) + + assert "stale" in json.loads(plugin_json.read_text())["lspServers"] + def test_multiple_stale_removed(self, tmp_path): - lsp_json = tmp_path / ".lsp.json" - lsp_json.write_text( + plugin_json = tmp_path / _CLAUDE_PROJECT_PLUGIN + plugin_json.parent.mkdir(parents=True) + plugin_json.write_text( json.dumps( { - "keep": {"command": "x"}, - "stale1": {"command": "y"}, - "stale2": {"command": "z"}, + "name": "apm-lsp", + "lspServers": { + "keep": {"command": "x"}, + "stale1": {"command": "y"}, + "stale2": {"command": "z"}, + }, } ) ) LSPIntegrator.remove_stale({"stale1", "stale2"}, project_root=tmp_path) - data = json.loads(lsp_json.read_text()) - assert set(data.keys()) == {"keep"} + data = json.loads(plugin_json.read_text()) + assert set(data["lspServers"]) == {"keep"} # =========================================================================== @@ -472,3 +796,74 @@ def test_collects_from_apm_yml(self, tmp_path): result = LSPIntegrator.collect_transitive(modules_dir, lock_path) assert len(result) == 1 assert result[0].name == "pyright" + assert result[0].resolved_by == dep.get_unique_key() + assert dep.get_unique_key() in result[0].approval_keys + assert "test-pkg#1.0.0" not in result[0].approval_keys + assert set(result[0].approval_keys) == { + dep.get_unique_key(), + f"{dep.get_unique_key()}#1.0.0", + } + + def test_rejects_symlinked_locked_package_manifest(self, tmp_path): + modules_dir = tmp_path / "apm_modules" + package_dir = modules_dir / "owner" / "repo" + package_dir.mkdir(parents=True) + outside = tmp_path / "outside-apm.yml" + outside.write_text("name: escaped\n", encoding="utf-8") + try: + (package_dir / "apm.yml").symlink_to(outside) + except OSError as exc: + pytest.skip(f"file symlinks unavailable: {exc}") + + from apm_cli.deps.lockfile import LockedDependency, LockFile + + lock_path = tmp_path / "apm.lock.yaml" + lock = LockFile() + lock.add_dependency(LockedDependency(repo_url="owner/repo", version="1.0.0")) + lock.save(lock_path) + + with pytest.raises(ValueError, match="manifest must not be a symlink"): + LSPIntegrator.collect_transitive(modules_dir, lock_path) + + def test_unlocked_fallback_does_not_invent_approval_keys(self, tmp_path): + pkg_dir = tmp_path / "apm_modules" / "ghe.example" / "owner" / "repo" + pkg_dir.mkdir(parents=True) + (pkg_dir / "apm.yml").write_text( + "name: enterprise-lsp\n" + "version: 1.0.0\n" + "dependencies:\n" + " lsp:\n" + " - name: pyright\n" + " command: pyright-langserver\n" + " extensionToLanguage:\n" + " .py: python\n", + encoding="utf-8", + ) + + result = LSPIntegrator.collect_transitive(tmp_path / "apm_modules") + + assert result[0].resolved_by == "ghe.example/owner/repo" + assert result[0].approval_keys == () + + +def test_skill_cleanup_preserves_reserved_claude_lsp_plugin(tmp_path: Path) -> None: + """Legacy skill cleanup must consume the LSP owner's reservation.""" + from apm_cli.integration.skill_support import clean_orphaned_skills + + skills_dir = tmp_path / ".claude" / "skills" + plugin = skills_dir / "apm-lsp" / ".claude-plugin" / "plugin.json" + plugin.parent.mkdir(parents=True) + plugin.write_text('{"name": "apm-lsp", "lspServers": {}}', encoding="utf-8") + orphan = skills_dir / "old-apm-skill" + orphan.mkdir() + + result = clean_orphaned_skills( + skills_dir, + set(), + project_root=tmp_path, + get_lockfile_owned_agent_skills=lambda _root: set(), + ) + + assert plugin.is_file() + assert not orphan.exists() + assert result == {"files_removed": 1, "errors": 0} diff --git a/tests/unit/scripts/test_architecture_runner.py b/tests/unit/scripts/test_architecture_runner.py index 9dddffa86..ede9534aa 100644 --- a/tests/unit/scripts/test_architecture_runner.py +++ b/tests/unit/scripts/test_architecture_runner.py @@ -619,6 +619,7 @@ def exiting_import( install-deployment-cached-claude-skill-metadata install-deployment-dependency-winner-selection install-deployment-deployment-frame-projection +install-deployment-executable-trust-context install-deployment-frozen-mutation-eligibility install-deployment-git-object-field-authority install-deployment-gitlab-facade-orchestration @@ -628,6 +629,8 @@ def exiting_import( install-deployment-local-bundle-policy-preflight install-deployment-local-identity-anchor install-deployment-locked-skill-subset-reconstruction +install-deployment-lsp-lifecycle +install-deployment-lsp-target-contract install-deployment-manifest-inheritance-includes install-deployment-marketplace-mutation-lock install-deployment-mcp-ownership-migration diff --git a/tests/unit/security/test_executables.py b/tests/unit/security/test_executables.py index 6535c23be..a4665b324 100644 --- a/tests/unit/security/test_executables.py +++ b/tests/unit/security/test_executables.py @@ -39,6 +39,7 @@ filter_mcp_by_allow_executables, is_any_type_approved, is_package_approved, + local_bundle_approval_key, more_severe_exec_status, parse_allow_executables, prompt_executable_approval, @@ -204,6 +205,39 @@ def test_marketplace_format(self) -> None: assert build_approval_key("ci-hooks@acme", "1.2.0") == "ci-hooks@acme#1.2.0" +class TestLocalBundleApprovalKey: + def test_hash_includes_normally_excluded_directories(self, tmp_path: Path) -> None: + payload = tmp_path / "__pycache__" / "server.py" + payload.parent.mkdir() + payload.write_text("first", encoding="utf-8") + first = local_bundle_approval_key("demo", "", tmp_path) + + payload.write_text("second", encoding="utf-8") + + assert local_bundle_approval_key("demo", "", tmp_path) != first + + def test_hash_rejects_symlinks(self, tmp_path: Path) -> None: + target = tmp_path / "target" + target.write_text("payload", encoding="utf-8") + link = tmp_path / "link" + try: + link.symlink_to(target) + except OSError as exc: + pytest.skip(f"file symlinks unavailable: {exc}") + + with pytest.raises(ValueError, match="contains a symlink"): + local_bundle_approval_key("demo", "", tmp_path) + + def test_locked_hash_uses_manifest_without_reading_payloads(self, tmp_path: Path) -> None: + lockfile = {"pack": {"bundle_files": {"extension.mjs": "a" * 64}}} + + with patch.object(Path, "read_bytes") as read_bytes: + key = local_bundle_approval_key("demo", "1", tmp_path, lockfile) + + read_bytes.assert_not_called() + assert key.startswith("demo#1@sha256:") + + # --------------------------------------------------------------------------- # scan_package_executables # --------------------------------------------------------------------------- @@ -295,6 +329,21 @@ def test_detects_lsp_from_apm_yml(self) -> None: assert EXEC_TYPE_LSP in decl.exec_types assert decl.lsp_details == ["pyright", "typescript-language-server"] + def test_preparsed_manifest_avoids_a_second_yaml_read(self, tmp_path) -> None: + manifest_data = { + "dependencies": {"lsp": [{"name": "pyright", "command": "pyright-langserver"}]} + } + with patch("apm_cli.utils.yaml_io.load_yaml") as load_yaml: + decl = scan_package_executables( + tmp_path, + "lsp-pkg", + "1.0", + manifest_data=manifest_data, + ) + + load_yaml.assert_not_called() + assert decl.lsp_count == 1 + def test_transitive_flag(self) -> None: with tempfile.TemporaryDirectory() as tmpdir: decl = scan_package_executables( @@ -651,8 +700,16 @@ def test_atomic_write_sets_owner_only_mode(self, tmp_path, monkeypatch) -> None: class _FakeMcpDep: """Minimal stand-in for an MCP dependency exposing ``.name``.""" - def __init__(self, name: str | None) -> None: + def __init__( + self, + name: str | None, + *, + resolved_by: str | None = None, + approval_keys: tuple[str, ...] = (), + ) -> None: self.name = name + self.resolved_by = resolved_by + self.approval_keys = approval_keys class _RecordingLogger: @@ -707,29 +764,105 @@ def test_unnamed_dep_is_fail_closed(self) -> None: class TestFilterLspFailClosed: - def test_project_allowed_slug_passes(self) -> None: + def test_root_lsp_passes_as_project_authored_content(self) -> None: deps = [_FakeMcpDep("pyright")] logger = _RecordingLogger() result = filter_lsp_by_allow_executables( deps, - {"pyright": {"lsp": True}}, + {}, logger, ) assert result == deps assert logger.warnings == [] - def test_unapproved_lsp_is_filtered(self) -> None: - deps = [_FakeMcpDep("pyright")] + def test_unapproved_declaring_package_is_filtered(self) -> None: + deps = [ + _FakeMcpDep( + "pyright", + resolved_by="evil/package", + approval_keys=("evil/package",), + ) + ] logger = _RecordingLogger() - result = filter_lsp_by_allow_executables(deps, {}, logger) + result = filter_lsp_by_allow_executables( + deps, + {}, + logger, + ) assert result == [] assert logger.warnings == [ - "Filtered 1 LSP server(s) whose executables are not trusted yet." + "Filtered 1 LSP server from 'evil/package': declaring package is " + "not trusted yet. Run 'apm policy explain evil/package'; approve it " + "only if policy permits." ] + assert "LSP server 'pyright' from 'evil/package'" in logger.verbose[0] + assert "if policy permits" in logger.verbose[0] + + def test_approval_is_bound_to_declarer_not_server_name(self) -> None: + approved = _FakeMcpDep( + "shared-name", + resolved_by="trusted/package", + approval_keys=("trusted/package",), + ) + impersonator = _FakeMcpDep( + "shared-name", + resolved_by="evil/package", + approval_keys=("evil/package",), + ) + logger = _RecordingLogger() + + result = filter_lsp_by_allow_executables( + [approved, impersonator], + {"trusted/package": {"lsp": True}}, + logger, + ) + + assert result == [approved] + + def test_unlocked_declarer_gets_lock_recovery_guidance(self) -> None: + dependency = _FakeMcpDep("pyright", resolved_by="ghe.example/owner/repo") + logger = _RecordingLogger() + + result = filter_lsp_by_allow_executables([dependency], {}, logger) + + assert result == [] + assert "regenerate apm.lock.yaml" in logger.verbose[0] + + def test_multiple_unlocked_declarers_get_plural_recovery_guidance(self) -> None: + dependencies = [ + _FakeMcpDep("pyright", resolved_by="ghe.example/owner/one"), + _FakeMcpDep("ruff", resolved_by="ghe.example/owner/two"), + ] + logger = _RecordingLogger() + + result = filter_lsp_by_allow_executables(dependencies, {}, logger) + + assert result == [] + assert "approve each package" in logger.warnings[0] + assert "LSP server 'pyright'" in logger.verbose[0] + assert "LSP server 'ruff'" in logger.verbose[1] + + def test_mixed_declarers_get_distinct_recovery_guidance(self) -> None: + dependencies = [ + _FakeMcpDep( + "pyright", + resolved_by="owner/locked", + approval_keys=("owner/locked",), + ), + _FakeMcpDep("ruff", resolved_by="owner/unlocked"), + ] + logger = _RecordingLogger() + + result = filter_lsp_by_allow_executables(dependencies, {}, logger) + + assert result == [] + warning = logger.warnings[0] + assert "policy explain owner/locked" in warning + assert "regenerate apm.lock.yaml" in warning @pytest.mark.parametrize( diff --git a/tests/unit/security/test_resolve_exec_decision.py b/tests/unit/security/test_resolve_exec_decision.py index 3e5fac08a..ae3002231 100644 --- a/tests/unit/security/test_resolve_exec_decision.py +++ b/tests/unit/security/test_resolve_exec_decision.py @@ -10,6 +10,7 @@ from apm_cli.security.executables import ( EXEC_TYPE_BIN, EXEC_TYPE_HOOKS, + EXEC_TYPE_LSP, EXEC_TYPE_MCP, LAYER_DEFAULT_DENY, LAYER_ENFORCE_DEGRADED, @@ -238,6 +239,29 @@ def test_default_deny_yields_empty_map(self): assert m is not None assert m == {} + def test_content_bound_grant_does_not_widen_to_package_name(self): + digest_key = f"{PKG}@sha256:{'a' * 64}" + ctx = _ctx(project_allow={digest_key: {EXEC_TYPE_LSP: True}}) + + effective = materialize_exec_map(ctx) + + assert effective == {digest_key: {EXEC_TYPE_LSP: True}} + assert resolve_exec_decision(ctx, digest_key, EXEC_TYPE_LSP).allowed is True + assert resolve_exec_decision(ctx, PKG, EXEC_TYPE_LSP).allowed is False + assert resolve_exec_decision(ctx, NAME, EXEC_TYPE_LSP).allowed is False + + def test_org_name_deny_still_blocks_content_bound_grant(self): + digest_key = f"{PKG}@sha256:{'a' * 64}" + ctx = _ctx( + project_allow={digest_key: {EXEC_TYPE_LSP: True}}, + org_deny=frozenset({NAME}), + ) + + effective = materialize_exec_map(ctx) + + assert effective == {} + assert resolve_exec_decision(ctx, digest_key, EXEC_TYPE_LSP).allowed is False + # ------------------------------------------------------------------- # exec_status_for_declaration: lockfile worst-case folding (Gap B) diff --git a/tests/utils/lifecycle_state.py b/tests/utils/lifecycle_state.py index cb225997b..93135afa1 100644 --- a/tests/utils/lifecycle_state.py +++ b/tests/utils/lifecycle_state.py @@ -500,7 +500,7 @@ def _mcp_state(lock: LockFile | None) -> dict[str, object]: def _empty_lsp_state() -> dict[str, object]: - return {"servers": [], "configs": {}} + return {"servers": [], "configs": {}, "target_servers": {}, "provenance": {}} def _lsp_state(lock: LockFile | None) -> dict[str, object]: @@ -509,6 +509,10 @@ def _lsp_state(lock: LockFile | None) -> dict[str, object]: return { "servers": sorted(lock.lsp_servers), "configs": dict(sorted(lock.lsp_configs.items())), + "target_servers": { + target: sorted(servers) for target, servers in sorted(lock.lsp_target_servers.items()) + }, + "provenance": dict(sorted(lock.lsp_config_provenance.items())), }