diff --git a/CHANGELOG.md b/CHANGELOG.md index edd8965eb..4c28ea28c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- `apm install` and `apm compile` now accept `--root DIR` to redirect every + generated artifact -- `apm_modules/`, `apm.lock.yaml`, `.gitignore`, and + integrated harness files (install) or `AGENTS.md` and per-target files + (compile) -- under `DIR`, while `apm.yml`, `.apm/`, and local-path + dependencies still resolve from the current working directory. This + mirrors `pip install --target` and `npm install --prefix`: useful for + scratch builds, CI artifact staging, and read-only source trees. `DIR` is + created if missing (refused under `--dry-run`); the redirect is reverted + on every exit path so it never leaks across invocations. `--root` is + rejected with `--global` (install) and `--watch` (compile). Thanks to + @srid (juspay) for the feature and original implementation. (closes #888, #1628) - `apm audit` can now ingest findings from external SARIF 2.1.0 scanners (e.g. NVIDIA SkillSpector or any SARIF-emitting tool) via `--external ` and `--external-sarif `, merging them into APM's own report diff --git a/docs/src/content/docs/reference/cli/compile.md b/docs/src/content/docs/reference/cli/compile.md index 6bf479a22..7cfc734f9 100644 --- a/docs/src/content/docs/reference/cli/compile.md +++ b/docs/src/content/docs/reference/cli/compile.md @@ -74,6 +74,7 @@ use `apm install` or `apm deps update` when you want shared | Flag | Description | |------|-------------| +| `--root DIR` | Redirect generated artifacts (AGENTS.md and per-target files) under `DIR` while `apm.yml`, `.apm/`, and `apm_modules/` still resolve from the current working directory. Mirrors `pip install --target`. `DIR` is created if missing. Not valid with `--watch`. | | `-o, --output PATH` | Output file path. Only applies in single-file mode (`--single-agents`). Default: `AGENTS.md`. | | `--single-agents` | Force single-file compilation (legacy). Writes one combined file at `--output` instead of a distributed per-directory target-file tree. Applies to every target that uses distributed placement. | | `--clean` | Remove orphaned AGENTS.md files no longer produced by the current primitive set. | @@ -151,6 +152,13 @@ Remove stale AGENTS.md files after deleting primitives: apm compile --clean ``` +Redirect generated files to a scratch directory (sources stay in `$PWD`): + +```bash +apm compile --root /tmp/agents-out --target copilot +ls /tmp/agents-out # AGENTS.md / per-target files; the source tree stays clean +``` + ## Watch mode `apm compile --watch` monitors the project for source changes and @@ -239,7 +247,7 @@ output. |------|---------| | 0 | Compilation succeeded (or `--validate` passed). | | 1 | No `apm.yml`, no primitives to compile, validation failure, hidden-Unicode finding, or compilation error. | -| 2 | Conflicting flags (e.g. `--all` combined with `--target`). | +| 2 | Conflicting flags (e.g. `--all` combined with `--target`, or `--root` combined with `--watch`). | ## Related diff --git a/docs/src/content/docs/reference/cli/install.md b/docs/src/content/docs/reference/cli/install.md index 6f80da180..0432f4de7 100644 --- a/docs/src/content/docs/reference/cli/install.md +++ b/docs/src/content/docs/reference/cli/install.md @@ -36,6 +36,12 @@ With no arguments it installs everything from `apm.yml`. With one or more `PACKA | `--verbose`, `-v` | off | Show per-file paths and full error context in the diagnostic summary. | | `--dev` | off | Add new packages to `devDependencies`. Dev deps install locally but are excluded from `apm pack` output. | +### Deploy location + +| Flag | Default | Description | +|---|---|---| +| `--root DIR` | `$PWD` | Redirect every write -- `apm_modules/`, `apm.lock.yaml`, `.gitignore`, and integrated harness files -- under `DIR`, while `apm.yml`, `.apm/`, and local-path dependencies still resolve from the current working directory. Mirrors `pip install --target` / `npm install --prefix`. `DIR` is created if missing (except under `--dry-run`, which refuses to create it). Not valid with `--global` (user scope), which exits `2`. | + ### Target selection | Flag | Default | Description | @@ -155,6 +161,17 @@ apm install --dry-run apm install microsoft/apm-sample-package --dry-run ``` +### Redirect writes to a scratch directory + +```bash +# Resolve apm.yml + local deps from this directory, but write +# apm_modules/, apm.lock.yaml, and harness files under /tmp/apm-out. +apm install --root /tmp/apm-out --target copilot + +# The source tree stays clean; the deploy root holds every artifact. +ls /tmp/apm-out # apm_modules/ apm.lock.yaml .github/ .gitignore +``` + ### Install a local bundle produced by `apm pack` ```bash @@ -176,7 +193,7 @@ apm install owner/skill-bundle --skill '*' # reset to all skills |---|---| | `0` | Success. All requested dependencies and local content deployed. | | `1` | Install failure: security scan blocked a critical finding, auth error, manifest write error, dependency resolution error, `--frozen` with a missing lockfile or a direct dependency absent from `apm.lock.yaml`, any reported install error (the diagnostic summary closes with `Installation failed with N error(s)`), or unhandled exception. `--force` does **not** suppress general install errors. The diagnostic summary names the cause. | -| `2` | Usage error: no deployment target detectable (no `--target`, no `targets:` in `apm.yml`, no harness signal in the project), `--ssh` and `--https` both passed, `--frozen` and `--update` both passed, or a Click flag conflict. | +| `2` | Usage error: no deployment target detectable (no `--target`, no `targets:` in `apm.yml`, no harness signal in the project), `--ssh` and `--https` both passed, `--frozen` and `--update` both passed, `--root` combined with `--global`, or a Click flag conflict. | ## Notes diff --git a/packages/apm-guide/.apm/skills/apm-usage/commands.md b/packages/apm-guide/.apm/skills/apm-usage/commands.md index d6edd9806..46ec35537 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/commands.md +++ b/packages/apm-guide/.apm/skills/apm-usage/commands.md @@ -10,7 +10,7 @@ | Command | Purpose | Key flags | |---------|---------|-----------| -| `apm install [PKGS...]` | Install APM and MCP dependencies (supports APM packages, Claude skills (SKILL.md), and plugin collections (plugin.json)) | `--update` (deprecated; prefer `apm update`) refresh refs, `--refresh` re-fetch all deps from upstream and re-resolve all ref pins, `--force` overwrite (does NOT refresh refs; use `apm update` for that), `--frozen` CI-safe install that fails fast when `apm.lock.yaml` is missing or out of sync with `apm.yml` (mutually exclusive with `--update`; structural presence check only -- use `apm audit` for SHA integrity), `--dry-run`, `--verbose`, `--only [apm\|mcp]`, `--target` (comma-separated, e.g. `--target claude,cursor`; highest-priority entry in the resolution chain `--target` > apm.yml `targets:` > auto-detect; `--target all` deprecated, see `apm compile --all`; use `copilot-cowork` with `--global` after `apm experimental enable copilot-cowork`), `--dev`, `-g` global, `--trust-transitive-mcp`, `--parallel-downloads N`, `--allow-insecure`, `--allow-insecure-host HOSTNAME`, `--skill NAME` install named skill(s) from SKILL_BUNDLE (repeatable; persisted in apm.yml; `'*'` resets to all), `--legacy-skill-paths` restore per-client skill dirs, `--mcp NAME` add MCP entry (NAME goes through the same `--target` > `targets:` > auto-detect resolver as APM packages, so a project whitelisting `targets: [copilot]` will not write `.cursor/mcp.json` even if `.cursor/` exists; `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 | +| `apm install [PKGS...]` | Install APM and MCP dependencies (supports APM packages, Claude skills (SKILL.md), and plugin collections (plugin.json)) | `--update` (deprecated; prefer `apm update`) refresh refs, `--refresh` re-fetch all deps from upstream and re-resolve all ref pins, `--force` overwrite (does NOT refresh refs; use `apm update` for that), `--frozen` CI-safe install that fails fast when `apm.lock.yaml` is missing or out of sync with `apm.yml` (mutually exclusive with `--update`; structural presence check only -- use `apm audit` for SHA integrity), `--dry-run`, `--verbose`, `--only [apm\|mcp]`, `--target` (comma-separated, e.g. `--target claude,cursor`; highest-priority entry in the resolution chain `--target` > apm.yml `targets:` > auto-detect; `--target all` deprecated, see `apm compile --all`; use `copilot-cowork` with `--global` after `apm experimental enable copilot-cowork`), `--dev`, `-g` global, `--trust-transitive-mcp`, `--parallel-downloads N`, `--allow-insecure`, `--allow-insecure-host HOSTNAME`, `--skill NAME` install named skill(s) from SKILL_BUNDLE (repeatable; persisted in apm.yml; `'*'` resets to all), `--legacy-skill-paths` restore per-client skill dirs, `--mcp NAME` add MCP entry (NAME goes through the same `--target` > `targets:` > auto-detect resolver as APM packages, so a project whitelisting `targets: [copilot]` will not write `.cursor/mcp.json` even if `.cursor/` exists; `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) | | `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 (accepts `owner/repo` or `name@marketplace`) | `--dry-run`, `-g` global | | `apm prune` | Remove orphaned packages | `--dry-run` | @@ -51,7 +51,7 @@ If no `--target`, no `targets:` in `apm.yml`, and no harness signal is present, | Command | Purpose | Key flags | |---------|---------|-----------| -| `apm compile` | Compile agent context | `-o` output, `-t` target (comma-separated; resolution chain `--target` > apm.yml `targets:` > auto-detect), `--all` compile for every canonical target (preferred over deprecated `--target all`), `--chatmode`, `--dry-run`, `--no-links`, `--watch`, `--validate`, `--single-agents`, `-v` verbose, `--local-only`, `--clean`, `--with-constitution/--no-constitution`, `--no-dedup` / `--force-instructions` (opt out of Claude deduplication) | +| `apm compile` | Compile agent context | `-o` output, `-t` target (comma-separated; resolution chain `--target` > apm.yml `targets:` > auto-detect), `--all` compile for every canonical target (preferred over deprecated `--target all`), `--chatmode`, `--dry-run`, `--no-links`, `--watch`, `--validate`, `--single-agents`, `-v` verbose, `--local-only`, `--clean`, `--with-constitution/--no-constitution`, `--no-dedup` / `--force-instructions` (opt out of Claude deduplication), `--root DIR` redirect generated artifacts under DIR while sources resolve from `$PWD` (mirrors `pip install --target`; not valid with `--watch`) | `apm compile --watch` live-reloads `apm.yml`: editing `target:` / `targets:` mid-session takes effect on the next file event without restarting the watcher. The CLI `--target` flag, when passed to `apm compile --watch`, still outranks `apm.yml`. Re-resolution is gated on the changed file's basename being `apm.yml`, so `.instructions.md` edits do not pay an extra resolver round-trip and a stray `backup_apm.yml` cannot trigger a reload. `--clean` is ignored in watch mode and the watcher prints an explicit `[!]` warning at startup (`--clean is ignored in watch mode; run 'apm compile --clean' separately to remove orphaned outputs.`); run `apm compile --clean` separately between watch sessions to remove orphans. diff --git a/src/apm_cli/commands/compile/cli.py b/src/apm_cli/commands/compile/cli.py index 67ea9a016..9ebfc397b 100644 --- a/src/apm_cli/commands/compile/cli.py +++ b/src/apm_cli/commands/compile/cli.py @@ -409,6 +409,19 @@ def _resolve_effective_target( help="Alias for --no-dedup.", hidden=True, ) +@click.option( + "--root", + "root", + type=click.Path(file_okay=False, resolve_path=True), + default=None, + metavar="DIR", + help=( + "Write AGENTS.md / CLAUDE.md outputs under DIR instead of $PWD; " + "sources (apm.yml, .apm/, project tree for placement scoring) " + "continue resolving from $PWD. Pairs with 'apm install --root' " + "for scratch-dir verification. Cannot be combined with --watch." + ), +) @click.pass_context def compile( ctx, @@ -427,6 +440,7 @@ def compile( legacy_skill_paths, compile_all, no_dedup, + root, ): """Compile APM context into distributed AGENTS.md files. @@ -464,11 +478,37 @@ def compile( # consumers running with -W default, which we have none of. logger.warning("'--target all' is deprecated; use '--all' instead.") + # --root + --watch is rejected: ``_watch_mode`` uses bare-relative + # paths (``Path(APM_DIR)``, ``AgentsCompiler(".")``) and the watch + # loop would scan the deploy root rather than the source tree. The + # flag combination has no real use case -- watch is interactive + # development; --root is for CI scratch-dir verification. + if root and watch: + raise click.UsageError("--root is not valid with --watch") + + # --root: see apm_cli.install.root_redirect.compile_root_redirect. + # Bracket the handler so writes land under *root* while sources keep + # resolving from the captured original $PWD via the source-root + # override. ``--dry-run`` is threaded through so the context manager + # skips the ``mkdir`` side-effect on previews. The manager is entered + # manually (rather than via ``with``) so the existing top-level + # try/except below does not need a 300-line re-indent; the matching + # ``finally`` at the end of the handler restores cwd + clears the + # override on every exit path (return, sys.exit, exception). + from ...core.scope import InstallScope, get_source_root + from ...install.root_redirect import compile_root_redirect + + _root_redirect = compile_root_redirect(root, dry_run=dry_run) + _root_redirect.__enter__() try: + # Source root: where apm.yml, .apm/, and the project tree are read + # from. Equals $PWD unless --root redirects writes elsewhere. + source_root = get_source_root(InstallScope.PROJECT) + # Check if this is an APM project first from pathlib import Path - if not Path(APM_YML_FILENAME).exists(): + if not (source_root / APM_YML_FILENAME).exists(): logger.error("Not an APM project - no apm.yml found") logger.progress(" To initialize an APM project, run:") logger.progress(" apm init") @@ -477,11 +517,11 @@ def compile( # Check if there are any instruction files to compile from ...compilation.constitution import find_constitution - apm_modules_exists = Path(APM_MODULES_DIR).exists() - constitution_exists = find_constitution(Path(".")).exists() + apm_modules_exists = (source_root / APM_MODULES_DIR).exists() + constitution_exists = find_constitution(source_root).exists() # Check if .apm directory has actual content - apm_dir = Path(APM_DIR) + apm_dir = source_root / APM_DIR local_apm_has_content = apm_dir.exists() and ( any(apm_dir.rglob("*.instructions.md")) or any(apm_dir.rglob("*.chatmode.md")) ) @@ -515,9 +555,9 @@ def compile( logger.start("Validating APM context...", symbol="gear") clear_discovery_cache() perf_stats.reset() - compiler = AgentsCompiler(".") + compiler = AgentsCompiler(".", source_dir=str(source_root)) try: - primitives = discover_primitives(".") + primitives = discover_primitives(str(source_root)) except Exception as e: logger.error(f"Failed to discover primitives: {e}") logger.progress(f" Error details: {type(e).__name__}") @@ -536,13 +576,13 @@ def compile( try: from ...models.apm_package import APMPackage - apm_pkg = APMPackage.from_apm_yml(Path(APM_YML_FILENAME)) + apm_pkg = APMPackage.from_apm_yml(source_root / APM_YML_FILENAME) mcp_count = len(apm_pkg.get_mcp_dependencies()) if mcp_count > 0: logger.progress(f" * {mcp_count} MCP dependencies") except Exception: pass - perf_stats.render_summary(logger, project_root=".") + perf_stats.render_summary(logger, project_root=str(source_root)) return # Watch mode @@ -593,7 +633,7 @@ def compile( from ...models.apm_package import APMPackage config_target = None - apm_yml_path = Path(APM_YML_FILENAME) + apm_yml_path = source_root / APM_YML_FILENAME if apm_yml_path.exists(): apm_pkg = APMPackage.from_apm_yml(apm_yml_path) config_target = apm_pkg.target @@ -637,7 +677,7 @@ def compile( # typed for Optional[str], and a frozenset config_target is already # handled by the branch above. detected_target, detection_reason = detect_target( - project_root=Path("."), + project_root=source_root, explicit_target=compile_target, config_target=compile_config_target if isinstance(compile_config_target, str) @@ -769,7 +809,7 @@ def _coerce_provenance_targets(value): # same process (tests, REPL). Mirrors run_install_pipeline. clear_discovery_cache() perf_stats.reset() - compiler = AgentsCompiler(".") + compiler = AgentsCompiler(".", source_dir=str(source_root)) result = compiler.compile(config, logger=logger) compile_has_critical = result.has_critical_security @@ -927,10 +967,10 @@ def _coerce_provenance_targets(value): "Compiled output contains critical hidden characters" " -- run 'apm audit' to inspect, 'apm audit --strip' to clean" ) - perf_stats.render_summary(logger, project_root=".") + perf_stats.render_summary(logger, project_root=str(source_root)) sys.exit(1) - perf_stats.render_summary(logger, project_root=".") + perf_stats.render_summary(logger, project_root=str(source_root)) except ImportError as e: logger.error(f"Compilation module not available: {e}") @@ -939,3 +979,7 @@ def _coerce_provenance_targets(value): except Exception as e: logger.error(f"Error during compilation: {e}") sys.exit(1) + finally: + # Restore cwd + clear the source-root override regardless of how + # the handler exits (return, sys.exit -> SystemExit, exception). + _root_redirect.__exit__(None, None, None) diff --git a/src/apm_cli/commands/install.py b/src/apm_cli/commands/install.py index bbebfdd55..4aca77791 100644 --- a/src/apm_cli/commands/install.py +++ b/src/apm_cli/commands/install.py @@ -1116,6 +1116,20 @@ def _handle_mcp_install( "local-bundle installs; passing --as without a local bundle path is rejected." ), ) +@click.option( + "--root", + "root", + type=click.Path(file_okay=False, resolve_path=True), + default=None, + metavar="DIR", + help=( + "Install into DIR instead of $PWD: apm_modules/, apm.lock.yaml, " + ".claude/, .codex/, .agents/, .opencode/ are written under DIR " + "while sources (apm.yml, .apm/, local-path packages) continue " + "resolving from $PWD. Mirrors 'pip install --target' / " + "'npm install --prefix'. Project scope only; not valid with --global." + ), +) @click.pass_context def install( # noqa: PLR0913 ctx, @@ -1152,6 +1166,7 @@ def install( # noqa: PLR0913 refresh, legacy_skill_paths, alias, + root, ): """Install APM and MCP dependencies from apm.yml (like npm install). @@ -1195,12 +1210,27 @@ def install( # noqa: PLR0913 "--frozen and --update are mutually exclusive. " "Use 'apm update' to refresh refs, then 'apm install --frozen' in CI." ) + # --root: see apm_cli.install.root_redirect.install_root_redirect. + # Conflicts with --global (user scope writes are anchored at $HOME + # and have no concept of an arbitrary deploy root). ``--dry-run`` is + # threaded through so the context manager skips the ``mkdir`` + # side-effect on previews. Entered manually (rather than via + # ``with``) so the existing top-level try/except/finally below does + # not need a full-body re-indent; the matching ``__exit__`` in that + # ``finally`` restores cwd + clears the source-root override on every + # exit path (return, sys.exit -> SystemExit, exception). + 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 + from ..install.root_redirect import install_root_redirect try: audit_override = resolve_audit_override_from_cli(no_audit, audit_mode) except ValueError as exc: raise click.UsageError(str(exc)) from exc + + _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 @@ -1592,6 +1622,11 @@ def install( # noqa: PLR0913 _rich_error(f"Error installing dependencies: {e}") sys.exit(1) finally: + # --root: restore cwd + clear the source-root override regardless + # of how the handler exits (return, sys.exit -> SystemExit, + # exception). Done first so cwd is back to $PWD before any + # best-effort summary rendering below. + _root_redirect.__exit__(None, None, None) # F5 (#1116): render minimal elapsed-time line on exit paths that # did not already render the full install summary. Best-effort: # never let a render failure mask the original exception/exit. diff --git a/src/apm_cli/compilation/agents_compiler.py b/src/apm_cli/compilation/agents_compiler.py index 35986c63b..e2c14f815 100644 --- a/src/apm_cli/compilation/agents_compiler.py +++ b/src/apm_cli/compilation/agents_compiler.py @@ -239,13 +239,21 @@ class CompilationResult: class AgentsCompiler: """Main compiler for generating AGENTS.md files.""" - def __init__(self, base_dir: str = "."): + def __init__(self, base_dir: str = ".", source_dir: str | None = None): """Initialize the compiler. Args: - base_dir (str): Base directory for compilation. Defaults to current directory. + base_dir (str): Base directory for compilation -- where AGENTS.md / + CLAUDE.md outputs are written and the relative root for + placement decisions. Defaults to the current directory. + source_dir (Optional[str]): Where primitives (``.apm/``, + ``apm_modules/``) and source files are discovered. Defaults to + ``base_dir`` for back-compat; set explicitly when ``apm + compile --root`` redirects writes but sources remain in + ``$PWD``. """ self.base_dir = Path(base_dir) + self.source_dir = Path(source_dir) if source_dir else self.base_dir self.warnings: list[str] = [] self.errors: list[str] = [] self._logger = None @@ -285,7 +293,7 @@ def compile( if config.local_only: # Use basic discovery for local-only mode primitives = discover_primitives( - str(self.base_dir), + str(self.source_dir), exclude_patterns=config.exclude, ) else: @@ -293,7 +301,7 @@ def compile( from ..primitives.discovery import discover_primitives_with_dependencies primitives = discover_primitives_with_dependencies( - str(self.base_dir), + str(self.source_dir), exclude_patterns=config.exclude, ) @@ -422,9 +430,14 @@ def _compile_distributed( errors = self.validate_primitives(primitives) self.errors.extend(errors) - # Create distributed compiler with exclude patterns + # Create distributed compiler with exclude patterns. source_dir + # carries through so primitive discovery + project-tree scoring + # honor `apm compile --root` (sources stay in $PWD, writes + # redirect to base_dir). distributed_compiler = DistributedAgentsCompiler( - str(self.base_dir), exclude_patterns=config.exclude + str(self.base_dir), + exclude_patterns=config.exclude, + source_dir=str(self.source_dir), ) # Skip instructions in AGENTS.md when they are already deployed to @@ -604,7 +617,7 @@ def _compile_claude_md( self.errors.extend(errors) # Create Claude formatter - claude_formatter = ClaudeFormatter(str(self.base_dir)) + claude_formatter = ClaudeFormatter(str(self.base_dir), source_dir=str(self.source_dir)) # Honor compilation.strategy=single-file (and the --single-agents flag) # by collapsing all instructions into a single root CLAUDE.md, mirroring @@ -622,7 +635,9 @@ def _compile_claude_md( from .distributed_compiler import DistributedAgentsCompiler distributed_compiler = DistributedAgentsCompiler( - str(self.base_dir), exclude_patterns=config.exclude + str(self.base_dir), + exclude_patterns=config.exclude, + source_dir=str(self.source_dir), ) # Analyze directory structure and determine placement directory_map = distributed_compiler.analyze_directory_structure( @@ -958,7 +973,10 @@ def validate_primitives(self, primitives: PrimitiveCollection) -> list[str]: for primitive in primitives.all_primitives(): primitive_errors = primitive.validate() if primitive_errors: - file_path = portable_relpath(primitive.file_path, self.base_dir) + # Source files live under source_dir; relativise display + # paths against it so `apm compile --root` doesn't render + # absolute or `../../` paths in warning messages. + file_path = portable_relpath(primitive.file_path, self.source_dir) for error in primitive_errors: # Treat validation errors as warnings instead of hard errors @@ -970,7 +988,7 @@ def validate_primitives(self, primitives: PrimitiveCollection) -> list[str]: primitive_dir = primitive.file_path.parent link_errors = validate_link_targets(primitive.content, primitive_dir) if link_errors: - file_path = portable_relpath(primitive.file_path, self.base_dir) + file_path = portable_relpath(primitive.file_path, self.source_dir) for link_error in link_errors: self.warnings.append(f"{file_path}: {link_error}") @@ -1007,8 +1025,12 @@ def _generate_template_data( Returns: TemplateData: Template data for generation. """ - # Build instructions content - instructions_content = build_conditional_sections(primitives.instructions) + # Build instructions content. source_dir keeps `` + # display paths relative to the user's working directory when + # `apm compile --root` redirects writes elsewhere. + instructions_content = build_conditional_sections( + primitives.instructions, source_dir=self.source_dir + ) # Metadata (version only; timestamp intentionally omitted for determinism) version = get_version() @@ -1157,7 +1179,12 @@ def _generate_copilot_root_instructions_content( sections.append("") for instruction in instructions: - rel_path = portable_relpath(instruction.file_path, self.base_dir) + # instruction.file_path is a source-tree file; relativise it + # against source_dir so `apm compile --root` never leaks + # `../../` or absolute deploy-relative paths into the + # `` provenance comments (sources stay in $PWD + # while writes redirect to base_dir). + rel_path = portable_relpath(instruction.file_path, self.source_dir) if config.source_attribution: sections.append(f"") sections.append(instruction.content.strip()) @@ -1362,7 +1389,10 @@ def _display_trace_info(self, distributed_result, primitives: PrimitiveCollectio for instruction in placement.instructions: source = getattr(instruction, "source", "local") - inst_path = portable_relpath(instruction.file_path, self.base_dir) + # instruction.file_path is a source-tree file; relativise + # against source_dir so `apm compile --root` produces + # human-readable paths in verbose output. + inst_path = portable_relpath(instruction.file_path, self.source_dir) self._log( "verbose_detail", diff --git a/src/apm_cli/compilation/claude_formatter.py b/src/apm_cli/compilation/claude_formatter.py index 6ec2361b5..7c107a83a 100644 --- a/src/apm_cli/compilation/claude_formatter.py +++ b/src/apm_cli/compilation/claude_formatter.py @@ -11,6 +11,7 @@ from typing import Dict, List, Optional, Set # noqa: F401, UP035 from ..primitives.models import Chatmode, Instruction, PrimitiveCollection +from ..utils.paths import resolve_base_and_source_dirs from ..version import get_version from .constants import BUILD_ID_PLACEHOLDER from .constitution import read_constitution @@ -62,16 +63,20 @@ class ClaudeFormatter: not included in CLAUDE.md (same as AGENTS.md behavior). """ - def __init__(self, base_dir: str = "."): + def __init__(self, base_dir: str = ".", source_dir: str | None = None): """Initialize the Claude formatter. Args: - base_dir (str): Base directory for compilation. + base_dir (str): Base directory for compilation -- where CLAUDE.md + outputs are written. Defaults to the current directory. + source_dir (Optional[str]): Where source primitives and the + constitution are read from. Defaults to ``base_dir`` for + back-compat; set explicitly when ``apm compile --root`` + redirects writes but sources remain in ``$PWD`` so that + ```` provenance comments render relative + to the user's working directory rather than the deploy root. """ - try: - self.base_dir = Path(base_dir).resolve() - except (OSError, FileNotFoundError): - self.base_dir = Path(base_dir).absolute() + self.base_dir, self.source_dir = resolve_base_and_source_dirs(base_dir, source_dir) self.warnings: builtins.list[str] = [] self.errors: builtins.list[str] = [] @@ -172,7 +177,7 @@ def _generate_placements( # Handle empty placement map with constitution or dependencies if not placement_map: - constitution = read_constitution(self.base_dir) + constitution = read_constitution(self.source_dir) dependencies = self._collect_dependencies() if constitution or dependencies: root_path = self.base_dir / "CLAUDE.md" @@ -298,7 +303,7 @@ def _generate_claude_content( # Constitution section (only for root CLAUDE.md) if placement.is_root: - constitution = read_constitution(self.base_dir) + constitution = read_constitution(self.source_dir) if constitution: sections.append("# Constitution") sections.append("") @@ -317,7 +322,7 @@ def _generate_claude_content( build_attributed_instructions( placement.instructions, placement.source_attribution, - self.base_dir, + self.source_dir, ) ) diff --git a/src/apm_cli/compilation/distributed_compiler.py b/src/apm_cli/compilation/distributed_compiler.py index f97f2b24b..e5fb00293 100644 --- a/src/apm_cli/compilation/distributed_compiler.py +++ b/src/apm_cli/compilation/distributed_compiler.py @@ -15,7 +15,7 @@ from ..output.formatters import CompilationFormatter from ..output.models import CompilationResults from ..primitives.models import Instruction, PrimitiveCollection -from ..utils.paths import portable_relpath +from ..utils.paths import portable_relpath, resolve_base_and_source_dirs from ..version import get_version from .constants import BUILD_ID_PLACEHOLDER from .context_optimizer import ContextOptimizer @@ -76,28 +76,52 @@ class CompilationResult: class DistributedAgentsCompiler: """Main compiler for generating distributed AGENTS.md files.""" - def __init__(self, base_dir: str = ".", exclude_patterns: builtins.list[str] | None = None): + def __init__( + self, + base_dir: str = ".", + exclude_patterns: builtins.list[str] | None = None, + source_dir: str | None = None, + ): """Initialize the distributed AGENTS.md compiler. Args: - base_dir (str): Base directory for compilation. - exclude_patterns (Optional[List[str]]): Glob patterns for directories to exclude. + base_dir (str): Base directory for compilation -- root used to + construct AGENTS.md write paths. Defaults to the current + directory. + exclude_patterns (Optional[List[str]]): Glob patterns for + directories to exclude. + source_dir (Optional[str]): Where primitives and the project + tree are scanned for placement scoring. Defaults to + ``base_dir`` for back-compat; set explicitly when ``apm + compile --root`` redirects writes but sources remain in + ``$PWD``. """ - try: - self.base_dir = Path(base_dir).resolve() - except (OSError, FileNotFoundError): - self.base_dir = Path(base_dir).absolute() + self.base_dir, self.source_dir = resolve_base_and_source_dirs(base_dir, source_dir) self.warnings: builtins.list[str] = [] self.errors: builtins.list[str] = [] self.total_files_written = 0 self.context_optimizer = ContextOptimizer( - str(self.base_dir), exclude_patterns=exclude_patterns + str(self.source_dir), exclude_patterns=exclude_patterns ) - self.link_resolver = UnifiedLinkResolver(self.base_dir) + self.link_resolver = UnifiedLinkResolver(self.source_dir) self.output_formatter = CompilationFormatter() self._placement_map = None + def _source_to_base(self, path: Path) -> Path: + """Map a path rooted at source_dir to the equivalent base_dir path. + + Returns *path* unchanged when source_dir == base_dir (the default + case) or when *path* is not under source_dir (defensive fallback). + """ + if self.source_dir == self.base_dir: + return path + try: + rel = path.resolve().relative_to(self.source_dir.resolve()) + except (ValueError, OSError): + return path + return self.base_dir / rel + def compile_distributed( self, primitives: PrimitiveCollection, config: dict | None = None ) -> CompilationResult: @@ -308,18 +332,26 @@ def determine_agents_placement( Returns: Dict[Path, List[Instruction]]: Optimized mapping of directory paths to instructions. """ - # Use the Context Optimization Engine for intelligent placement + # Use the Context Optimization Engine for intelligent placement. + # The optimizer scans source_dir, so the returned placement keys + # are rooted at source_dir; translate them to base_dir below so + # writes land at the deploy root when source_dir != base_dir + # (`apm compile --root`). optimized_placement = self.context_optimizer.optimize_instruction_placement( instructions, verbose=debug, enable_timing=debug, # Enable timing when debug mode is on ) + if optimized_placement and self.source_dir != self.base_dir: + optimized_placement = { + self._source_to_base(p): v for p, v in optimized_placement.items() + } # Special case: if no instructions but constitution exists, create root placement if not optimized_placement: from .constitution import find_constitution - constitution_path = find_constitution(Path(self.base_dir)) + constitution_path = find_constitution(Path(self.source_dir)) if constitution_path.exists(): # Create an empty placement for the root directory to enable verbose output optimized_placement = {Path(self.base_dir): []} @@ -369,7 +401,7 @@ def generate_distributed_agents_files( if not placement_map: from .constitution import find_constitution - constitution_path = find_constitution(Path(self.base_dir)) + constitution_path = find_constitution(Path(self.source_dir)) if constitution_path.exists(): # Create a root placement for constitution-only projects root_path = Path(self.base_dir) @@ -562,7 +594,7 @@ def _generate_agents_content( build_attributed_instructions( placement.instructions, placement.source_attribution, - self.base_dir, + self.source_dir, ) ) diff --git a/src/apm_cli/compilation/template_builder.py b/src/apm_cli/compilation/template_builder.py index 573ab7a8e..376a6f5a5 100644 --- a/src/apm_cli/compilation/template_builder.py +++ b/src/apm_cli/compilation/template_builder.py @@ -126,11 +126,19 @@ def _emit(instruction: Instruction) -> list[str]: ) -def build_conditional_sections(instructions: list[Instruction]) -> str: +def build_conditional_sections( + instructions: list[Instruction], + source_dir: Path | None = None, +) -> str: """Build sections grouped by applyTo patterns. Args: - instructions (List[Instruction]): List of instruction primitives. + instructions: List of instruction primitives. + source_dir: Root used to compute display-relative paths in + ```` comments. Defaults to ``Path.cwd()``; + callers using ``apm compile --root`` should pass the source + root so attribution paths render relative to the user's + working directory rather than the deploy target. Returns: str: Formatted conditional sections content. @@ -138,12 +146,16 @@ def build_conditional_sections(instructions: list[Instruction]) -> str: if not instructions: return "" - cwd = Path.cwd() + # ``source_dir`` is the project source root. Defaults to ``Path.cwd()``; + # callers using ``apm compile --root`` pass the captured ``$PWD`` so + # ```` paths render against the user's working + # directory rather than the deploy target. + relpath_root = source_dir if source_dir is not None else Path.cwd() def emit(instruction: Instruction) -> list[str]: try: if instruction.file_path.is_absolute(): - relative_path = portable_relpath(instruction.file_path, cwd) + relative_path = portable_relpath(instruction.file_path, relpath_root) else: relative_path = str(instruction.file_path) except (ValueError, OSError): @@ -156,7 +168,7 @@ def emit(instruction: Instruction) -> list[str]: "", ] - sections = render_instructions_block(instructions, base_dir=cwd, emit_instruction=emit) + sections = render_instructions_block(instructions, base_dir=relpath_root, emit_instruction=emit) return "\n".join(sections) diff --git a/src/apm_cli/core/scope.py b/src/apm_cli/core/scope.py index d5f1f610c..6b8dc5b68 100644 --- a/src/apm_cli/core/scope.py +++ b/src/apm_cli/core/scope.py @@ -9,10 +9,26 @@ User-scope support varies by target -- see ``TargetProfile.user_supported`` in ``apm_cli.integration.targets`` for the canonical registry. + +Source-root override +-------------------- +``set_source_root_override`` pins the source root (where ``apm.yml`` +and local-path packages resolve from) to an explicit directory. The +``apm install --root`` flow uses this together with ``os.chdir(root)`` +so existing call-sites that hardcode ``Path.cwd()`` automatically +resolve to the deploy root, while sources continue to read from the +captured original working directory. + +Write helpers (:func:`get_deploy_root`, :func:`get_apm_dir`, +:func:`get_modules_dir`, :func:`get_lockfile_dir`) intentionally do +NOT consult the override -- after ``chdir`` they already point to the +deploy root. Source helpers (:func:`get_source_root`, +:func:`get_manifest_path`) consult it. """ from __future__ import annotations +import contextvars from enum import Enum from pathlib import Path from typing import List # noqa: F401, UP035 @@ -37,17 +53,77 @@ class InstallScope(Enum): USER = "user" +# --------------------------------------------------------------------------- +# Source-root override (managed by the ``--root`` flag handlers) +# --------------------------------------------------------------------------- +# +# Backed by ``contextvars.ContextVar`` rather than a module-global so the +# override is isolated per execution context: nested / sequential CLI +# invocations and test cases never observe each other's pins, and there is +# no cross-test leakage to reset. ``ContextVar`` is NOT inherited by +# ``ThreadPoolExecutor`` download workers, which is intentional and safe: +# the override is consulted only by the source-read helpers below +# (:func:`get_source_root` / :func:`get_manifest_path`), all of which run +# on the main thread during the resolve phase. Deploy-side redirection is +# carried by the process-global ``os.chdir`` in ``install_root_redirect``, +# which every thread observes regardless of this ContextVar. + + +_SOURCE_ROOT_OVERRIDE: contextvars.ContextVar[Path | None] = contextvars.ContextVar( + "apm_source_root_override", default=None +) + + +def set_source_root_override(path: Path | None) -> None: + """Pin the project-scope source root to *path*. + + Used by ``apm install --root`` (and any command that ``chdir``s + into a deploy directory) to remember the user's original working + directory so :func:`get_source_root` and :func:`get_manifest_path` + keep reading sources from there. + + Pass ``None`` to clear the override. Command handlers should + always clear in a ``try/finally`` block so the override never + leaks across CLI invocations. + """ + _SOURCE_ROOT_OVERRIDE.set(path.resolve() if path is not None else None) + + +def get_source_root_override() -> Path | None: + """Return the active source-root override, or ``None`` when unset.""" + return _SOURCE_ROOT_OVERRIDE.get() + + # --------------------------------------------------------------------------- # Path resolution # --------------------------------------------------------------------------- +def get_source_root(scope: InstallScope) -> Path: + """Return the directory used to read project sources. + + Project scope: the active source-root override when set, otherwise + ``Path.cwd()``. User scope: ``Path.home()``. + + Sources resolved from this root: ``apm.yml``, ``.apm/`` local + primitives, and the resolution base for local-path package + references. + """ + if scope is InstallScope.USER: + return Path.home() + override = _SOURCE_ROOT_OVERRIDE.get() + if override is not None: + return override + return Path.cwd() + + def get_deploy_root(scope: InstallScope) -> Path: """Return the root used to construct deployment paths. - For project scope this is ``Path.cwd()``. - For user scope this is ``Path.home()`` so that integrators produce - paths like ``~/.claude/commands/``. + For project scope this is ``Path.cwd()`` -- callers that want to + redirect deployment should ``chdir`` into the target directory and + use :func:`set_source_root_override` to remember the original + source root. For user scope this is ``Path.home()``. """ if scope is InstallScope.USER: return Path.home() @@ -55,9 +131,9 @@ def get_deploy_root(scope: InstallScope) -> Path: def get_apm_dir(scope: InstallScope) -> Path: - """Return the directory that holds APM metadata (manifest, lockfile, modules). + """Return the directory that holds APM metadata (lockfile, modules). - * Project scope: ``/`` + * Project scope: ``/`` (the active deploy root) * User scope: ``~/.apm/`` """ if scope is InstallScope.USER: @@ -73,14 +149,39 @@ def get_modules_dir(scope: InstallScope) -> Path: def get_manifest_path(scope: InstallScope) -> Path: - """Return the ``apm.yml`` path for *scope*.""" + """Return the ``apm.yml`` path for *scope*. + + The manifest is a SOURCE -- its location follows + :func:`get_source_root`, which honours + :func:`set_source_root_override`. This keeps ``apm install --root`` + reading the manifest from the user's original working directory + rather than from the (typically empty) deploy root. + + User scope delegates to :func:`get_apm_dir` so the user-level + location can't drift between this helper and the dir helper. + """ from ..constants import APM_YML_FILENAME - return get_apm_dir(scope) / APM_YML_FILENAME + if scope is InstallScope.USER: + return get_apm_dir(scope) / APM_YML_FILENAME + return get_source_root(scope) / APM_YML_FILENAME def get_lockfile_dir(scope: InstallScope) -> Path: - """Return the directory containing the lockfile for *scope*.""" + """Return the directory containing the lockfile for *scope*. + + The lockfile is a WRITE -- it is regenerated by ``apm install`` and + co-located with ``apm_modules/``, so it follows + :func:`get_apm_dir` (which honours + :func:`set_source_root_override` via ``Path.cwd()`` after + ``install_root_redirect`` chdirs into the deploy root). + + Contrast with :func:`get_manifest_path`, which follows + :func:`get_source_root` because the manifest is a source file. If + you add a new metadata helper, pick the side deliberately: source + files go through :func:`get_source_root`; generated / deployed + artifacts go through :func:`get_apm_dir`. + """ return get_apm_dir(scope) diff --git a/src/apm_cli/install/context.py b/src/apm_cli/install/context.py index e6c8a50cb..c11750727 100644 --- a/src/apm_cli/install/context.py +++ b/src/apm_cli/install/context.py @@ -34,9 +34,17 @@ class InstallContext: # ------------------------------------------------------------------ project_root: Path apm_dir: Path - - # ------------------------------------------------------------------ - # Inputs: populated by the caller from CLI args / APMPackage + # Source root for reads (``apm.yml``, ``.apm/``, local-path + # packages). Equal to ``project_root`` unless ``apm install --root`` + # redirects writes -- then ``source_root`` stays at ``$PWD`` while + # ``project_root`` is the override. + # + # Resolved at the CLI boundary (``run_install_pipeline``). When a + # caller does not pass it, ``__post_init__`` defaults it to + # ``project_root`` -- the correct value whenever ``--root`` is absent. + # Phases always read ``ctx.source_root`` (never re-derive from + # ``project_root``); only the ``--root`` path makes the two diverge. + source_root: Path | None = None # ------------------------------------------------------------------ apm_package: Any = None # APMPackage update_refs: bool = False @@ -176,3 +184,11 @@ class InstallContext: # Legacy skill paths opt-out (convergence ยง3) # ------------------------------------------------------------------ legacy_skill_paths: bool = False # --legacy-skill-paths flag or APM_LEGACY_SKILL_PATHS env + + def __post_init__(self) -> None: + # ``source_root`` defaults to ``project_root`` (the correct value + # whenever ``apm install --root`` is not used). Only the --root + # CLI path passes a distinct source_root; every other caller and + # test gets source_root == project_root for free. + if self.source_root is None: + self.source_root = self.project_root diff --git a/src/apm_cli/install/phases/integrate.py b/src/apm_cli/install/phases/integrate.py index 1463949dc..66037e913 100644 --- a/src/apm_cli/install/phases/integrate.py +++ b/src/apm_cli/install/phases/integrate.py @@ -346,6 +346,7 @@ def _integrate_root_project( diagnostics=diagnostics, logger=logger, scope=ctx.scope, + source_root=ctx.source_root, ctx=ctx, ) diff --git a/src/apm_cli/install/phases/resolve.py b/src/apm_cli/install/phases/resolve.py index 17d2d9c54..c8c151501 100644 --- a/src/apm_cli/install/phases/resolve.py +++ b/src/apm_cli/install/phases/resolve.py @@ -394,6 +394,11 @@ def run(ctx: InstallContext) -> None: # This matches the original code's closure over function-level locals. scope = ctx.scope project_root = ctx.project_root + # Local-path package references in apm.yml are relative to the + # manifest's location (source_root), not the deploy override. + # source_root is required on InstallContext; equals project_root + # when --root is not used. + source_root = ctx.source_root # --refresh implies re-resolution of all refs (but does NOT discard # lockfile entries for packages not in the manifest, unlike --update # which may restructure the whole graph). @@ -540,10 +545,16 @@ def download_callback(dep_ref, modules_dir, parent_chain="", parent_pkg=None): # Anchor relative paths on the *declaring* package's source # directory when available (#857). Falls back to project_root # for direct deps and for parents that predate source_path. + # Direct deps from the root project anchor at ``source_root`` + # (which equals ``project_root`` unless ``apm install --root`` + # redirects writes -- then it stays at $PWD). Transitive + # deps from a parent local package anchor at that package's + # source_path, which is already an absolute path and not + # affected by ``--root``. base_dir = ( parent_pkg.source_path if parent_pkg is not None and parent_pkg.source_path is not None - else project_root + else source_root ) result_path = _copy_local_package( dep_ref, @@ -694,7 +705,20 @@ def download_callback(dep_ref, modules_dir, parent_chain="", parent_pkg=None): download_callback=download_callback, ) - dependency_graph = resolver.resolve_dependencies(ctx.apm_dir) + # Resolver reads ``/apm.yml``. Preserve the original + # ``ctx.apm_dir`` anchor for every non-``--root`` install (zero + # behavior change: USER -> ``~/.apm``, PROJECT -> deploy root == cwd). + # When ``ctx.source_root`` differs from ``ctx.project_root`` (set by + # ``apm install --root`` via the pipeline), the manifest read diverges + # to ``ctx.source_root`` ($PWD) so sources keep resolving from the + # user's working directory while writes land under the deploy root. + # Using the ctx field (rather than the global ContextVar) makes this + # branch reachable for any caller that sets source_root directly. + # ``apm_modules_dir`` is already pinned on the resolver above, so + # this arg selects only where ``apm.yml`` is read -- never where + # ``apm_modules/`` is written. + manifest_anchor = ctx.source_root if ctx.source_root != ctx.project_root else ctx.apm_dir + dependency_graph = resolver.resolve_dependencies(manifest_anchor) ctx.dependency_graph = dependency_graph # Fold remote-parent local_path rejections into ``callback_failures`` so diff --git a/src/apm_cli/install/pipeline.py b/src/apm_cli/install/pipeline.py index 90a164e6f..5688a46d3 100644 --- a/src/apm_cli/install/pipeline.py +++ b/src/apm_cli/install/pipeline.py @@ -17,6 +17,25 @@ * Symbols on the ``commands/install`` module that phases access via ``_install_mod.X`` stay as re-exports there -- this module does NOT duplicate those re-exports. + +Source-vs-deploy root convention +-------------------------------- +:class:`InstallContext` carries two roots; phases must pick the +correct one or ``apm install --root`` silently produces wrong paths +(the bug surfaces only when ``project_root != source_root``). + +* ``ctx.source_root`` -- read sources here (``apm.yml``, ``.apm/`` + primitives, local-path packages). Equal to ``$PWD`` regardless of + ``--root``. +* ``ctx.project_root`` / ``ctx.apm_dir`` -- write deploy artifacts + here (``apm_modules/``, ``apm.lock.yaml``, ``.claude/``, ``.codex/``, + etc.). Becomes the ``--root`` target when set. + +Convention: a phase that *reads* an existing project file uses +``source_root``; a phase that *writes* anything uses ``project_root`` +(or the helper that already does -- e.g. :func:`get_apm_dir`). When +a new field is added to :class:`InstallContext`, the source-vs-write +side must be an explicit, documented choice -- not implicit. """ from __future__ import annotations @@ -366,7 +385,7 @@ def run_install_pipeline( # noqa: PLR0913, RUF100 _perf_stats.reset() clear_discovery_cache() - from ..core.scope import InstallScope, get_apm_dir, get_deploy_root + from ..core.scope import InstallScope, get_apm_dir, get_deploy_root, get_source_root if scope is None: scope = InstallScope.PROJECT @@ -375,13 +394,16 @@ def run_install_pipeline( # noqa: PLR0913, RUF100 dev_apm_deps = apm_package.get_dev_apm_dependencies() all_apm_deps = apm_deps + dev_apm_deps - project_root = get_deploy_root(scope) + project_root = get_deploy_root(scope) # write target + source_root = get_source_root(scope) # source reads (apm.yml, .apm/) apm_dir = get_apm_dir(scope) - # Check whether the project root itself has local .apm/ primitives (#714). + # Check whether the source root has local .apm/ primitives (#714). + # Sources resolve from $PWD even when --root redirects writes, so the + # check uses source_root rather than project_root. from apm_cli.install.phases.local_content import _project_has_root_primitives - _root_has_local_primitives = _project_has_root_primitives(project_root) + _root_has_local_primitives = _project_has_root_primitives(source_root) # Read old local deployed files from the existing lockfile so the # post-deps-local phase can run stale cleanup even when no current @@ -417,6 +439,7 @@ def run_install_pipeline( # noqa: PLR0913, RUF100 ctx = InstallContext( project_root=project_root, apm_dir=apm_dir, + source_root=source_root, apm_package=apm_package, update_refs=update_refs, verbose=verbose, diff --git a/src/apm_cli/install/root_redirect.py b/src/apm_cli/install/root_redirect.py new file mode 100644 index 000000000..e29954e1e --- /dev/null +++ b/src/apm_cli/install/root_redirect.py @@ -0,0 +1,102 @@ +"""``--root`` (deploy-root redirection) support for ``apm install`` / ``compile``. + +The flag lets users install into an arbitrary directory while keeping +sources in ``$PWD`` -- the precedent is ``pip install --target`` and +``npm install --prefix``. Implementation strategy: + +1. ``os.chdir(root)`` so every site that hardcodes ``Path.cwd()`` / + ``os.getcwd()`` (notably the MCP adapters in + :mod:`apm_cli.adapters.client`) automatically resolves to the deploy + root. Refactoring those sites to use scope helpers would touch a + long tail of files; the chdir trick is contained. +2. :func:`apm_cli.core.scope.set_source_root_override` pins the original + working directory so ``apm.yml``, ``.apm/``, and local-path package + resolution keep reading from ``$PWD``. + +Both effects are reverted on exit so global state never leaks across +CLI invocations (test runners, REPL sessions, embedded callers). The +``finally`` block is defensive: it restores the original working +directory even if that directory has since been removed, and it always +clears the source-root override regardless of how the body exited. +""" + +from __future__ import annotations + +import contextlib +import os +from collections.abc import Iterator +from contextlib import contextmanager +from pathlib import Path + + +@contextmanager +def install_root_redirect( + root: str | os.PathLike | None, + *, + dry_run: bool = False, +) -> Iterator[None]: + """Redirect deploy-side writes into *root* for the wrapped block. + + When *root* is ``None`` or empty, this is a no-op so callers can + wrap unconditionally. + + When set, captures the current working directory as the source + root, ``chdir``s into *root*, and restores both on exit (success + or exception). + + ``dry_run`` controls whether *root* is created when missing. In + write mode the directory is created (mirroring ``pip install + --target`` and ``npm install --prefix`` UX). In dry-run the + context manager refuses to mutate the filesystem -- if *root* + does not exist a ``click.UsageError`` is raised so the preview + cannot silently create directories on disk. + """ + if not root: + yield + return + + from ..core.scope import set_source_root_override + + target = Path(root) + if dry_run: + if not target.exists(): + import click + + raise click.UsageError( + f"--root {target} does not exist. " + "Create the directory before --dry-run, or drop --dry-run " + "to let install/compile create it." + ) + else: + # ``resolve()`` canonicalises the path (expands ``..`` components, + # makes it absolute, and follows any symlinks) before + # ``mkdir(parents=True)``, giving us a stable absolute path that + # matches what ``os.chdir`` will record as the new cwd. + target = target.resolve() + target.mkdir(parents=True, exist_ok=True) + original = Path.cwd() + set_source_root_override(original) + os.chdir(target) + try: + yield + finally: + # Restore both halves of the redirect unconditionally. If the + # original directory was removed while inside the block, leave + # the process where it is rather than crashing the command on + # the way out -- but always clear the override so it cannot + # leak into the next invocation. + with contextlib.suppress(OSError): + os.chdir(original) + set_source_root_override(None) + + +# ``apm compile --root`` and ``apm install --root`` need exactly the +# same chdir + source-root-pin pair: both commands write into *root* +# while reading sources from the captured ``$PWD``. The alias keeps +# them on a single implementation so the two flags can never drift. +# +# Split the alias into its own ``contextmanager`` only if compile +# develops needs that install doesn't (e.g. compile-only environment +# tweaks, an output-only sandbox). Until then, sharing prevents +# silent divergence. +compile_root_redirect = install_root_redirect diff --git a/src/apm_cli/install/services.py b/src/apm_cli/install/services.py index 919f4e1d9..8f5f88abb 100644 --- a/src/apm_cli/install/services.py +++ b/src/apm_cli/install/services.py @@ -463,6 +463,7 @@ def integrate_local_content( diagnostics: DiagnosticCollector, logger: InstallLogger | None = None, scope: InstallScope | None = None, + source_root: Path | None = None, ctx: InstallContext | None = None, ) -> dict: """Integrate primitives from the project's own .apm/ directory. @@ -475,20 +476,33 @@ def integrate_local_content( intentionally ignored (it describes the project itself, not a deployable skill). + Args: + project_root: Deploy root -- where ``.claude/``, ``.codex/``, + etc. are written. Also used to compute relative paths for + tracking deployed files. + source_root: Where to discover the synthetic local package's + ``.apm/`` content. Defaults to ``project_root`` when not + provided. When ``apm install --root`` is in play, + ``source_root`` stays at ``$PWD`` while ``project_root`` + points to the override. + Returns a dict with integration counters and deployed file paths, same shape as ``integrate_package_primitives()``. """ from ..models.apm_package import APMPackage, PackageInfo, PackageType + if source_root is None: + source_root = project_root + local_pkg = APMPackage( name="_local", version="0.0.0", - package_path=project_root, + package_path=source_root, source="local", ) local_info = PackageInfo( package=local_pkg, - install_path=project_root, + install_path=source_root, package_type=PackageType.APM_PACKAGE, ) diff --git a/src/apm_cli/install/sources.py b/src/apm_cli/install/sources.py index 2956becdb..47fa4254d 100644 --- a/src/apm_cli/install/sources.py +++ b/src/apm_cli/install/sources.py @@ -196,12 +196,15 @@ def acquire(self) -> Materialization | None: ) return None - # Determine the anchor for relative ``local_path`` (#857). For direct - # deps from the root project this is project_root. For transitive - # deps declared inside another local package, it is the parent - # package's source directory -- captured during resolve via - # ``ctx.dep_base_dirs``. - base_dir = getattr(ctx, "dep_base_dirs", {}).get(dep_key) or ctx.project_root + # Determine the anchor for relative ``local_path`` (#857). For + # direct deps from the root project this is ``ctx.source_root`` + # (which equals ``ctx.project_root`` unless ``apm install --root`` + # redirects writes -- then it stays at $PWD). For transitive + # deps declared inside another local package, the parent's + # source directory was captured during resolve via + # ``ctx.dep_base_dirs`` -- it is already absolute, so ``--root`` + # has nothing to do. + base_dir = getattr(ctx, "dep_base_dirs", {}).get(dep_key) or ctx.source_root result_path = _copy_local_package( dep_ref, install_path, diff --git a/src/apm_cli/utils/paths.py b/src/apm_cli/utils/paths.py index f084bf6cd..2cf0519b1 100644 --- a/src/apm_cli/utils/paths.py +++ b/src/apm_cli/utils/paths.py @@ -25,3 +25,26 @@ def portable_relpath(path: Path, base: Path) -> str: return path.resolve().as_posix() except (OSError, RuntimeError): return path.as_posix() + + +def resolve_base_and_source_dirs( + base_dir: str | Path, source_dir: str | Path | None +) -> tuple[Path, Path]: + """Resolve a compiler's write-root and source-root into absolute Paths. + + ``base_dir`` is where outputs are written; ``source_dir`` is where + primitives are read. ``source_dir`` defaults to ``base_dir`` for + back-compat and only diverges under ``apm compile --root`` (writes + redirected, sources stay in ``$PWD``). Resolution falls back to + ``absolute()`` when the path does not yet exist on disk. + """ + + def _resolve(value: str | Path) -> Path: + try: + return Path(value).resolve() + except (OSError, FileNotFoundError): + return Path(value).absolute() + + resolved_base = _resolve(base_dir) + resolved_source = resolved_base if source_dir is None else _resolve(source_dir) + return resolved_base, resolved_source diff --git a/tests/integration/test_compile_root_redirect_e2e.py b/tests/integration/test_compile_root_redirect_e2e.py new file mode 100644 index 000000000..b18d84f24 --- /dev/null +++ b/tests/integration/test_compile_root_redirect_e2e.py @@ -0,0 +1,100 @@ +"""End-to-end coverage for ``apm compile --root DIR``. + +The headline promise of the ``--root`` flag (issue #888) is a clean +split: sources resolve from ``$PWD`` while every generated artifact is +written under ``DIR``. These tests pin that contract end-to-end through +a real subprocess (no in-process Click harness) so a regression in the +chdir + source-root-override mechanism surfaces as a behavioral diff, +not just a unit-level assertion. + +Two invariants are load-bearing: + +* **byte-identical** -- the artifact written under ``--root`` is + identical to the one a plain ``apm compile`` would have produced in + ``$PWD``; ``--root`` only moves *where* output lands, never *what* is + generated. +* **no source-tree pollution + no leakage** -- the source tree stays + clean, and a subsequent plain ``apm compile`` (no ``--root``) once + again writes into ``$PWD``, proving the process-global override did + not leak across invocations. +""" + +from __future__ import annotations + +import subprocess +import sys +from pathlib import Path + +_COMPILE = [sys.executable, "-m", "apm_cli.cli", "compile"] + + +def _make_project(root: Path) -> None: + (root / "apm.yml").write_text("name: root-redirect-e2e\nversion: 0.1.0\n", encoding="utf-8") + instructions = root / ".apm" / "instructions" + instructions.mkdir(parents=True) + (instructions / "demo.instructions.md").write_text( + '---\ndescription: Demo guide\napplyTo: "**"\n---\n\n' + "# Demo\n\nHello from the source tree.\n", + encoding="utf-8", + ) + + +def _run(args: list[str], cwd: Path) -> subprocess.CompletedProcess: + return subprocess.run([*_COMPILE, *args], cwd=str(cwd), capture_output=True, text=True) + + +def test_root_redirect_is_byte_identical_and_keeps_sources_in_pwd( + tmp_path: Path, +) -> None: + src = tmp_path / "src" + src.mkdir() + _make_project(src) + + # Baseline: plain compile writes AGENTS.md into the source tree. + baseline = _run([], cwd=src) + assert baseline.returncode == 0, baseline.stderr or baseline.stdout + baseline_agents = src / "AGENTS.md" + assert baseline_agents.exists(), "baseline compile did not emit AGENTS.md" + baseline_bytes = baseline_agents.read_bytes() + baseline_agents.unlink() + + # --root: sources still read from ``src`` ($PWD), writes land in deploy. + deploy = tmp_path / "deploy" + deploy.mkdir() + redirected = _run(["--root", str(deploy)], cwd=src) + assert redirected.returncode == 0, redirected.stderr or redirected.stdout + + # Source tree stays clean -- no artifact leaked back into $PWD. + assert not (src / "AGENTS.md").exists(), "--root leaked AGENTS.md into $PWD" + + # Output landed under the deploy root and is byte-identical. + deployed_agents = deploy / "AGENTS.md" + assert deployed_agents.exists(), "--root did not emit AGENTS.md under DIR" + assert deployed_agents.read_bytes() == baseline_bytes, ( + "--root output diverged from baseline; the flag must only change " + "WHERE output lands, never WHAT is generated" + ) + + +def test_root_override_does_not_leak_into_next_invocation(tmp_path: Path) -> None: + src = tmp_path / "src" + src.mkdir() + _make_project(src) + + deploy = tmp_path / "deploy" + deploy.mkdir() + + first = _run(["--root", str(deploy)], cwd=src) + assert first.returncode == 0, first.stderr or first.stdout + assert (deploy / "AGENTS.md").exists() + assert not (src / "AGENTS.md").exists() + + # A subsequent plain compile (separate process, but the override is a + # process-global so this also guards the in-process unwind contract) + # must once again write into $PWD, not the previous deploy root. + second = _run([], cwd=src) + assert second.returncode == 0, second.stderr or second.stdout + assert (src / "AGENTS.md").exists(), ( + "plain compile after --root failed to write into $PWD -- " + "the source-root override leaked across invocations" + ) diff --git a/tests/integration/test_install_root_redirect_e2e.py b/tests/integration/test_install_root_redirect_e2e.py new file mode 100644 index 000000000..b764b1662 --- /dev/null +++ b/tests/integration/test_install_root_redirect_e2e.py @@ -0,0 +1,85 @@ +"""End-to-end coverage for ``apm install --root DIR``. + +Companion to ``test_compile_root_redirect_e2e``. Where the compile test +pins the byte-identical artifact promise, this test pins the *resolver* +half of the ``--root`` contract (issue #888): when the source-root +override is active, dependency manifests resolve from ``$PWD`` while the +``apm_modules/`` tree, lockfile, and integrated harness files are all +written under ``DIR``. + +This is the ``source_root != project_root`` path the review panel +flagged as uncovered: with no override, the resolver anchors the +manifest read on the (cwd-equal) project root; only under ``--root`` +does the manifest anchor diverge to ``$PWD`` while writes follow the +chdir into ``DIR``. + +A local ``path:`` dependency keeps the test hermetic -- no network, no +git, no registry. +""" + +from __future__ import annotations + +import subprocess +import sys +from pathlib import Path + +_INSTALL = [sys.executable, "-m", "apm_cli.cli", "install"] + + +def _make_local_dep(pkg: Path) -> None: + instructions = pkg / ".apm" / "instructions" + instructions.mkdir(parents=True) + (pkg / "apm.yml").write_text( + "name: dep-pkg\ndescription: local dep\nversion: 0.0.1\n", + encoding="utf-8", + ) + (instructions / "dep.instructions.md").write_text( + '---\ndescription: Dep guide\napplyTo: "**"\n---\n\n# Dep\n\nFrom the dependency.\n', + encoding="utf-8", + ) + + +def test_install_root_resolves_sources_from_pwd_and_writes_under_root( + tmp_path: Path, +) -> None: + pkg = tmp_path / "pkg" + _make_local_dep(pkg) + + consumer = tmp_path / "consumer" + consumer.mkdir() + (consumer / "apm.yml").write_text( + "name: consumer\n" + "description: consumer\n" + "version: 0.0.1\n" + "dependencies:\n" + " apm:\n" + f" - path: {pkg}\n", + encoding="utf-8", + ) + + deploy = tmp_path / "deploy" + deploy.mkdir() + + result = subprocess.run( + [*_INSTALL, "--root", str(deploy), "--target", "copilot"], + cwd=str(consumer), + capture_output=True, + text=True, + ) + assert result.returncode == 0, result.stderr or result.stdout + + # Writes landed under the deploy root: lockfile, modules tree, and the + # integrated harness file all live in ``deploy``. + assert (deploy / "apm.lock.yaml").exists(), "lockfile not written under --root" + assert (deploy / "apm_modules" / "_local" / "pkg").is_dir(), ( + "materialised dependency not written under --root" + ) + assert (deploy / ".github" / "instructions" / "dep.instructions.md").exists(), ( + "integrated harness file not written under --root" + ) + + # The source tree stays clean: the resolver read the manifest from + # ``$PWD`` but never wrote install artifacts back into it. + assert not (consumer / "apm_modules").exists(), "apm_modules leaked into $PWD" + assert not (consumer / "apm.lock.yaml").exists(), "lockfile leaked into $PWD" + assert not (consumer / ".github").exists(), "harness files leaked into $PWD" diff --git a/tests/unit/install/test_architecture_invariants.py b/tests/unit/install/test_architecture_invariants.py index 6127488f6..127df878f 100644 --- a/tests/unit/install/test_architecture_invariants.py +++ b/tests/unit/install/test_architecture_invariants.py @@ -200,12 +200,20 @@ def test_install_py_under_legacy_budget(): single override-resolution call. All glue at the handler boundary; the precedence logic lives in ``apm_cli/core/install_audit.py`` and the phase itself in ``apm_cli/install/phases/audit.py``. + + Issue #888 (``--root DIR`` deploy redirection) raised 2045 -> 2085 + to add the ``--root`` Click option plus its signature param, the + ``--root`` + ``--global`` UsageError guard, and the + ``install_root_redirect`` bracket (manual ``__enter__`` + a single + ``__exit__`` in the existing ``finally``). All glue at the handler + boundary; the chdir + source-root-override mechanism lives in + ``apm_cli/install/root_redirect.py`` and ``apm_cli/core/scope.py``. """ install_py = Path(__file__).resolve().parents[3] / "src" / "apm_cli" / "commands" / "install.py" assert install_py.is_file() n = _line_count(install_py) - assert n <= 2045, ( - f"commands/install.py grew to {n} LOC (budget 2045). " + assert n <= 2085, ( + f"commands/install.py grew to {n} LOC (budget 2085). " "Do NOT trim cosmetically -- engage the python-architecture skill " "(.apm/skills/python-architecture/SKILL.md) and propose an " "extraction into apm_cli/install/." diff --git a/tests/unit/install/test_root_redirect.py b/tests/unit/install/test_root_redirect.py new file mode 100644 index 000000000..1181a4326 --- /dev/null +++ b/tests/unit/install/test_root_redirect.py @@ -0,0 +1,144 @@ +"""Unit tests for ``install_root_redirect`` / ``compile_root_redirect``. + +The redirect mutates two pieces of process-global state: + +* the working directory (``os.chdir(root)``) +* the source-root override on :mod:`apm_cli.core.scope` + +Both must be restored on every exit path -- success, exception, early +return -- so concurrent CLI invocations and embedded test runners do +not leak state into one another. + +These tests are deliberately small and self-contained; they exercise +the context manager directly (no Click harness) so a regression that +breaks the unwind shows up here before any downstream test starts +seeing flaky cwd / pytest-tmpdir interactions. +""" + +from __future__ import annotations + +import os +from pathlib import Path + +import click +import pytest + +from apm_cli.core.scope import get_source_root_override +from apm_cli.install.root_redirect import ( + compile_root_redirect, + install_root_redirect, +) + + +@pytest.fixture(autouse=True) +def _restore_cwd(): + """Snapshot + restore CWD around each test. + + Failures in the redirect logic could leave us inside ``tmp_path`` + after the test exits; that breaks every subsequent test that + depends on the repo root. The snapshot here is belt-and-braces + insurance -- if a regression slips past the assertion that the + redirect restored CWD, this fixture catches it before the next + test gets confused. + """ + original = Path.cwd() + yield + if Path.cwd() != original: + os.chdir(original) + + +def test_noop_when_root_is_none(tmp_path): + """``root=None`` is a no-op: cwd unchanged, no override set.""" + cwd_before = Path.cwd() + assert get_source_root_override() is None + + with install_root_redirect(None): + assert Path.cwd() == cwd_before + assert get_source_root_override() is None + + assert Path.cwd() == cwd_before + assert get_source_root_override() is None + + +def test_noop_when_root_is_empty_string(): + """Empty string is treated like ``None`` so callers can pass falsy.""" + cwd_before = Path.cwd() + + with install_root_redirect(""): + assert Path.cwd() == cwd_before + assert get_source_root_override() is None + + assert get_source_root_override() is None + + +def test_chdir_and_override_on_entry(tmp_path): + """When *root* is set, cwd moves into it and override pins original.""" + cwd_before = Path.cwd() + + with install_root_redirect(tmp_path): + assert Path.cwd().resolve() == tmp_path.resolve() + assert get_source_root_override() == cwd_before.resolve() + + assert Path.cwd() == cwd_before + assert get_source_root_override() is None + + +def test_restore_on_exception(tmp_path): + """An exception inside the block must still restore cwd + override.""" + cwd_before = Path.cwd() + + class _Boom(RuntimeError): + pass + + with pytest.raises(_Boom): + with install_root_redirect(tmp_path): + assert Path.cwd().resolve() == tmp_path.resolve() + assert get_source_root_override() == cwd_before.resolve() + raise _Boom("simulated install failure") + + assert Path.cwd() == cwd_before + assert get_source_root_override() is None + + +def test_creates_target_dir_when_missing(tmp_path): + """Normal mode creates *root* if absent (mirrors pip --target UX).""" + deploy = tmp_path / "fresh" + assert not deploy.exists() + + with install_root_redirect(deploy): + assert deploy.is_dir() + + assert deploy.is_dir() + + +def test_dry_run_refuses_to_create_target(tmp_path): + """``dry_run=True`` must NOT create *root* -- raises UsageError.""" + deploy = tmp_path / "missing" + assert not deploy.exists() + + with pytest.raises(click.UsageError, match="does not exist"): + with install_root_redirect(deploy, dry_run=True): + pytest.fail("body should not execute") + + assert not deploy.exists(), "dry-run leaked a mkdir on disk" + + +def test_dry_run_works_when_target_exists(tmp_path): + """When *root* already exists, ``dry_run=True`` enters cleanly.""" + cwd_before = Path.cwd() + + with install_root_redirect(tmp_path, dry_run=True): + assert Path.cwd().resolve() == tmp_path.resolve() + assert get_source_root_override() == cwd_before.resolve() + + assert Path.cwd() == cwd_before + assert get_source_root_override() is None + + +def test_compile_alias_is_install_redirect(): + """The compile alias must be the same callable, not a copy. + + A copy would let one drift from the other on future edits; the + intentional aliasing prevents that silent divergence. + """ + assert compile_root_redirect is install_root_redirect