From f1a605862d586773ee6e74ee37bf201a01722d4c Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Wed, 9 Sep 2026 10:54:28 +0200 Subject: [PATCH 01/10] fix(models): restore installed Git package source identity Extract canonical identity restoration from #2820 at 824b17058826aef375c0446085197a81e1418dd8. This restores source identity, not acquisition trust; local and registry sources remain unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/apm_cli/models/apm_package.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/apm_cli/models/apm_package.py b/src/apm_cli/models/apm_package.py index b00e786c0e..71a9817c8c 100644 --- a/src/apm_cli/models/apm_package.py +++ b/src/apm_cli/models/apm_package.py @@ -843,6 +843,12 @@ def has_primitives(self) -> bool: return False +def restore_installed_package_source(package: APMPackage, dep_ref: DependencyReference) -> None: + """Restore Git acquisition provenance after loading installed authored metadata.""" + if dep_ref.source in (None, "git"): + package.source = dep_ref.to_github_url() + + def build_installed_package_info( dep_ref: DependencyReference, apm_modules_dir: Path ) -> PackageInfo | None: @@ -865,6 +871,8 @@ def build_installed_package_info( if not package: return None + restore_installed_package_source(package, dep_ref) + return PackageInfo( package=package, install_path=install_path, From cc2c309ff26a34c9721742ce081cb12a852f25e0 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Wed, 9 Sep 2026 11:09:35 +0200 Subject: [PATCH 02/10] fix(install): preserve generated aggregate ownership and rebuilds Extract the complete aggregate runtime and lifecycle proof from #2820 at 824b17058826aef375c0446085197a81e1418dd8 onto current main. Preserve shared final digests, source-backed survivor/root reconstruction, safe refusal receipts and proven-removal projection cleanup without activating corrective v0.2. Acquisition-source cached caller and its routing witness belong to the combined local/audit successor. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../owners/install-deployment.json | 6 +- ...install_base_integrator_and_contraction.py | 134 +++++++ .../checks/install_deployment_analyzers.py | 7 + src/apm_cli/commands/uninstall/cli.py | 35 +- src/apm_cli/commands/uninstall/engine.py | 88 +++++ .../commands/uninstall/lockfile_state.py | 15 + src/apm_cli/core/deployment_ledger.py | 16 + src/apm_cli/core/deployment_state.py | 33 +- src/apm_cli/install/helpers/security_scan.py | 10 +- src/apm_cli/install/manifest_reconcile.py | 1 + src/apm_cli/install/phases/lockfile.py | 70 +++- src/apm_cli/install/services.py | 6 +- .../integration/instruction_integrator.py | 38 +- .../test_architecture_copilot_aggregate.py | 78 ++++ .../test_architecture_owner_rule_mutations.py | 8 + .../test_copilot_aggregate_uninstall.py | 351 ++++++++++++++++++ .../test_lockfile_cross_package_reconcile.py | 116 ++++++ .../test_install_copilot_user_instructions.py | 2 +- .../unit/scripts/test_architecture_runner.py | 1 + tests/unit/test_aggregate_installed_source.py | 24 ++ tests/unit/test_uninstall_engine_helpers.py | 48 +++ 21 files changed, 1056 insertions(+), 31 deletions(-) create mode 100644 tests/integration/test_architecture_copilot_aggregate.py create mode 100644 tests/integration/test_copilot_aggregate_uninstall.py create mode 100644 tests/unit/test_aggregate_installed_source.py diff --git a/.apm/architecture/owners/install-deployment.json b/.apm/architecture/owners/install-deployment.json index a0872223aa..eb86983f9d 100644 --- a/.apm/architecture/owners/install-deployment.json +++ b/.apm/architecture/owners/install-deployment.json @@ -19,8 +19,8 @@ "id": "deployment-provenance-state", "decision": "Deployment provenance / state", "owner": "deployment_ledger.py", - "selectors": ["src/apm_cli/core/deployment_ledger.py"], - "guards": ["install-deployment-provenance-state"] + "selectors": ["src/apm_cli/core/deployment_ledger.py", "src/apm_cli/core/deployment_state.py"], + "guards": ["install-deployment-provenance-state", "install-deployment-copilot-aggregate"] }, { "id": "target-scoped-deployed-file-contraction", @@ -106,7 +106,7 @@ "id": "file-level-deploy-sync-cleanup", "decision": "File-level deploy / sync / cleanup", "owner": "BaseIntegrator (see integrators.instructions.md)", - "selectors": ["src/apm_cli/integration/base_integrator.py"], + "selectors": ["src/apm_cli/integration/base_integrator.py", "src/apm_cli/integration/instruction_integrator.py"], "guards": ["install-deployment-base-integrator"] }, { 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 59e0936ebe..05160e92aa 100644 --- a/scripts/architecture_linter/checks/install_base_integrator_and_contraction.py +++ b/scripts/architecture_linter/checks/install_base_integrator_and_contraction.py @@ -39,6 +39,140 @@ _GUARD_BASE_INTEGRATOR = "install-deployment-base-integrator" +_GUARD_COPILOT_AGGREGATE = "install-deployment-copilot-aggregate" + + +def check_copilot_aggregate(provider: FactsProvider) -> tuple[Violation, ...]: + """Keep aggregate lifecycle wiring at its integration and provenance owners.""" + routes = ( + ( + "src/apm_cli/install/phases/lockfile.py", + "_attach_deployed_files", + {"aggregate_paths", "reconcile_package_claims", "merge_aggregate_records"}, + ), + ( + "src/apm_cli/install/phases/lockfile.py", + "_preserve_existing_local_state", + {"aggregate_paths", "replace_legacy_owner"}, + ), + ( + "src/apm_cli/commands/uninstall/engine.py", + "_sync_integrations_after_uninstall", + { + "aggregate_paths", + "cleanup_snapshot", + "integrate_local_content", + "integrate_package_primitives", + "finalize_install_result", + }, + ), + ( + "src/apm_cli/integration/instruction_integrator.py", + "sync_for_target", + {"aggregate_paths", "remove_stale_deployed_files"}, + ), + ( + "src/apm_cli/models/apm_package.py", + "build_installed_package_info", + {"restore_installed_package_source"}, + ), + ( + "src/apm_cli/models/apm_package.py", + "restore_installed_package_source", + {"to_github_url"}, + ), + ( + "src/apm_cli/commands/uninstall/cli.py", + "uninstall", + {"reconcile_uninstall_deployment_state"}, + ), + ( + "src/apm_cli/commands/uninstall/lockfile_state.py", + "reconcile_uninstall_deployment_state", + {"MaterializationResult", "reconcile", "apply_to_lockfile"}, + ), + ) + findings = [] + for path, function, required in routes: + facts, errors = _facts_for(provider, path, _GUARD_COPILOT_AGGREGATE) + findings.extend(errors) + if errors: + continue + definitions = [ + definition for definition in facts.definitions if definition.name == function + ] + calls = { + call.qualname.rsplit(".", 1)[-1] + for call in facts.calls + if any( + definition.line <= call.line <= definition.end_line for definition in definitions + ) + } + missing = required - calls + if missing: + findings.append( + _summary( + _GUARD_COPILOT_AGGREGATE, + path, + f"{function} must route aggregate decisions through: {', '.join(sorted(missing))}", + ) + ) + # Successful cleanup facts must cross both adapters, not merely coexist + # with calls to the owner. Use the shared index (no second parse/walk). + forwardings = ( + ( + "src/apm_cli/commands/uninstall/engine.py", + "IntegrationCleanupOutcome", + "removed_aggregate_paths", + ), + ( + "src/apm_cli/commands/uninstall/cli.py", + "reconcile_uninstall_deployment_state", + "integration_cleanup", + ), + ) + for path, callee, source in forwardings: + index = provider.tree_index(path) + if index is None or index.root is None: + continue + forwarded = any( + isinstance(node, ast.Call) + and isinstance(node.func, ast.Name) + and node.func.id == callee + and any( + keyword.arg == "removed_paths" and source in _loaded_names(index, keyword.value) + for keyword in node.keywords + ) + for node in index.walk(index.root) + ) + if not forwarded: + findings.append( + _summary( + _GUARD_COPILOT_AGGREGATE, + path, + f"{callee} must forward confirmed removed_paths from {source}", + ) + ) + path = "src/apm_cli/commands/uninstall/lockfile_state.py" + index = provider.tree_index(path) + if index is not None and index.root is not None: + removed_proof = any( + isinstance(node, ast.Attribute) + and node.attr == "REMOVED" + and isinstance(node.value, ast.Name) + and node.value.id == "MaterializationStatus" + for node in index.walk(index.root) + ) + if not removed_proof: + findings.append( + _summary( + _GUARD_COPILOT_AGGREGATE, + path, + "Confirmed cleanup must supply canonical MaterializationStatus.REMOVED evidence", + ) + ) + return tuple(findings) + def _body_has_re(body: Sequence[str], pattern: re.Pattern[str]) -> bool: """Return whether any captured body line matches `pattern`.""" diff --git a/scripts/architecture_linter/checks/install_deployment_analyzers.py b/scripts/architecture_linter/checks/install_deployment_analyzers.py index d58844e0e7..0c105c66ad 100644 --- a/scripts/architecture_linter/checks/install_deployment_analyzers.py +++ b/scripts/architecture_linter/checks/install_deployment_analyzers.py @@ -15,9 +15,11 @@ from scripts.architecture_linter.checks.install_base_integrator_and_contraction import ( _GUARD_BASE_INTEGRATOR, + _GUARD_COPILOT_AGGREGATE, _GUARD_PROVENANCE, _GUARD_TARGET_CONTRACTION, check_base_integrator, + check_copilot_aggregate, check_provenance_state, check_target_file_contraction, ) @@ -72,6 +74,11 @@ from scripts.architecture_linter.models import Rule RULES: tuple[Rule, ...] = ( + _rule( + _GUARD_COPILOT_AGGREGATE, + "Generated Copilot aggregates use canonical ownership, cleanup and source reconstruction.", + check_copilot_aggregate, + ), _rule( _GUARD_PACKAGE_TARGET, "Restriction-only package target authorization has one owner (install/target_filter.py).", diff --git a/src/apm_cli/commands/uninstall/cli.py b/src/apm_cli/commands/uninstall/cli.py index 5638a8c01a..2c6ba704ef 100644 --- a/src/apm_cli/commands/uninstall/cli.py +++ b/src/apm_cli/commands/uninstall/cli.py @@ -14,6 +14,7 @@ from ...models.apm_package import APMPackage from ...utils.path_security import PathTraversalError from .engine import ( + AggregateIntegrationError, IntegrationCleanupOutcome, LocalSurvivorRefreshError, MCPUninstallCleanupError, @@ -89,11 +90,12 @@ def _sync_integrations_for_manifest( failed_paths=[], error_count=0, ) + recovery_command = "apm install --global" if user_scope else "apm install" try: apm_package = APMPackage.from_apm_yml(manifest_path) except Exception as manifest_err: logger.warning("Integration cleanup did not finish.") - logger.warning("Run 'apm install' to resync remaining integrations.") + logger.warning(f"Run '{recovery_command}' to resync remaining integrations.") logger.verbose_detail( f"Integration cleanup skipped: {type(manifest_err).__name__}: {manifest_err}" ) @@ -114,8 +116,30 @@ def _sync_integrations_for_manifest( None, ) except Exception as sync_err: - logger.warning("Integration cleanup did not finish.") - logger.warning("Run 'apm install' to resync remaining integrations.") + if isinstance(sync_err, AggregateIntegrationError): + from ...utils.diagnostics import printable_ascii_text + + logger.warning(str(sync_err)) + logger.warning( + "Package removal and declaration removal completed; integration cleanup is incomplete." + ) + prefix = "~/" if user_scope else "" + for path in sorted(sync_err.paths): + logger.warning(f"Managed path: {printable_ascii_text(prefix + path)}") + if sync_err.phase == "cleanup": + logger.warning( + "Back up and move the refused generated file out of the managed path " + "before regeneration; do not restore the stale multi-owner file." + ) + source_path = "~/.apm/instructions/" if user_scope else ".apm/instructions/" + logger.warning( + f"Keep genuine authoring in {source_path}. Repair unsafe paths before retrying." + ) + else: + logger.warning("Fix the reported source errors before retrying.") + else: + logger.warning("Integration cleanup did not finish.") + logger.warning(f"Run '{recovery_command}' to resync remaining integrations.") logger.verbose_detail(f"Integration cleanup failed: {type(sync_err).__name__}: {sync_err}") logger.verbose_detail(traceback.format_exc().rstrip()) return default_outcome, sync_err @@ -582,6 +606,11 @@ def uninstall(ctx, packages, dry_run, verbose, global_): deploy_root=deploy_root, all_deployed_files=all_deployed_files, surviving_deployed_files=surviving_deployed_files, + removed_paths=( + integration_cleanup.removed_paths + if integration_cleanup.complete and integration_cleanup_error is None + else frozenset() + ), fully_refreshed_dependency_keys=refreshed_survivor_keys, ) or lockfile_updated diff --git a/src/apm_cli/commands/uninstall/engine.py b/src/apm_cli/commands/uninstall/engine.py index 4349dace8d..0e4bafb20d 100644 --- a/src/apm_cli/commands/uninstall/engine.py +++ b/src/apm_cli/commands/uninstall/engine.py @@ -1080,6 +1080,7 @@ class IntegrationCleanupOutcome: deployed_files: dict[str, list[str]] failed_paths: list[str] error_count: int + removed_paths: frozenset[str] = frozenset() @property def complete(self) -> bool: @@ -1087,6 +1088,20 @@ def complete(self) -> bool: return self.error_count == 0 +class AggregateIntegrationError(RuntimeError): + """Safe public aggregate failure facts, without raw source exception text.""" + + def __init__(self, phase: str, paths: frozenset[str]) -> None: + self.phase = phase + self.paths = paths + cause = ( + "managed content changed or could not be safely removed" + if phase == "cleanup" + else "source validation or integration failed" + ) + super().__init__(f"Instruction aggregate {phase} failed: {cause}.") + + def _native_hook_state_exists(project_root: Path, targets: list[object]) -> bool: """Return whether cleanup may rewrite a merged native-hook config.""" from ...integration.hook_integrator import _APM_HOOKS_SIDECAR, _MERGE_HOOK_TARGETS @@ -1126,6 +1141,7 @@ def _sync_integrations_after_uninstall( """ from ...install.services import ( IntegratorBundle, + integrate_local_content, integrate_package_primitives, ) from ...install.target_filter import resolve_effective_package_targets @@ -1140,6 +1156,21 @@ def _sync_integrations_after_uninstall( _resolved_targets = resolve_targets( project_root, user_scope=user_scope, explicit_target=_explicit ) + from ...core.deployment_ledger import DeploymentLedgerCodec + from ...integration.instruction_integrator import InstructionIntegrator + + aggregate_paths = InstructionIntegrator.aggregate_paths(_resolved_targets) + aggregate_rebuilds = aggregate_paths.intersection(all_deployed_files) + cleanup_hashes = dict(deployed_file_hashes or {}) + if lockfile is not None and aggregate_paths: + # Include surviving claims: older installations recorded only the + # last contributor, so the selected dependency can have no path. + aggregate_snapshot = DeploymentLedgerCodec.cleanup_snapshot( + lockfile, {".", *lockfile.dependencies} + ) + aggregate_rebuilds |= aggregate_paths.intersection(aggregate_snapshot.paths) + cleanup_hashes.update(aggregate_snapshot.hashes) + all_deployed_files.update(aggregate_rebuilds) require_valid_survivors = bool(all_deployed_files) or _native_hook_state_exists( project_root, _resolved_targets, @@ -1212,6 +1243,7 @@ def _sync_integrations_after_uninstall( counts = {entry.counter_key: 0 for entry in _dispatch.values()} package_deployed_files: dict[str, list[str]] = {} + removed_aggregate_paths: set[str] = set() # Phase 1: Remove all APM-deployed files # Per-target sync for primitives with sync_for_target @@ -1232,13 +1264,27 @@ def _sync_integrations_after_uninstall( if _buckets is not None: _bucket_key = BaseIntegrator.partition_bucket_key(_prim_name, _target.name) _managed_subset = _buckets.get(_bucket_key, set()) + aggregate_sync = _prim_name == "instructions" and bool( + InstructionIntegrator.aggregate_paths([_target]) + ) + sync_options = ( + {"managed_file_hashes": cleanup_hashes, "diagnostics": logger.diagnostics} + if aggregate_sync + else {} + ) result = _integrators[_prim_name].sync_for_target( _target, apm_package, project_root, managed_files=_managed_subset, + **sync_options, ) counts[_entry.counter_key] += result.get("files_removed", 0) + if aggregate_sync: + removed_aggregate_paths.update(result.get("removed_paths", ())) + if aggregate_sync and result.get("errors", 0): + logger.render_summary() + raise AggregateIntegrationError("cleanup", aggregate_rebuilds) # Skills (multi-target, handled by SkillIntegrator) # Check both target root_dir and deploy_root for skill directories @@ -1377,6 +1423,7 @@ def _sync_integrations_after_uninstall( _rebuild_scope = InstallScope.USER if user_scope else InstallScope.PROJECT _allow_executables = getattr(apm_package, "allow_executables", None) reintegration_diagnostics = DiagnosticCollector() + reintegration_errors = 0 for dep_ref, pkg_info, authorized_targets in target_survivor_plan: dep_key = dep_ref.get_unique_key() deployed_files = package_deployed_files.setdefault(dep_key, []) @@ -1396,9 +1443,11 @@ def _sync_integrations_after_uninstall( allow_executables=_allow_executables, trust_bin=False, bin_skip_reason_override="not_retrusted_on_uninstall", + show_force_hint=False, ) deployed_files.extend(integration_result["deployed_files"]) except Exception as exc: + reintegration_errors += 1 pkg_id = _dependency_public_label(dep_ref) logger.warning( f"Best-effort re-integration skipped for {pkg_id}. " @@ -1408,12 +1457,51 @@ def _sync_integrations_after_uninstall( f" Re-integration error: {_reintegration_error_detail(dep_ref, exc)}" ) + if aggregate_rebuilds: + from dataclasses import replace + + # Rebuild only root instructions, not unrelated first-party primitives. + # Root sources come from the deployment root at user scope, exactly as + # install does; never adopt arbitrary text from the generated output. + root_targets = [ + replace(target, primitives={"instructions": target.primitives["instructions"]}) + for target in _resolved_targets + if InstructionIntegrator.aggregate_paths([target]).intersection(aggregate_rebuilds) + ] + root_result = integrate_local_content( + project_root, + targets=root_targets, + prompt_integrator=_integrator_bundle.prompt, + agent_integrator=_integrator_bundle.agent, + skill_integrator=_integrator_bundle.skill, + instruction_integrator=_integrator_bundle.instruction, + command_integrator=_integrator_bundle.command, + hook_integrator=_integrator_bundle.hook, + force=False, + managed_files=None, + diagnostics=reintegration_diagnostics, + logger=logger, + scope=_rebuild_scope, + show_force_hint=False, + ) + package_deployed_files["."] = root_result["deployed_files"] + reintegration_diagnostics.render_summary() + if aggregate_rebuilds: + from ...install.outcome import finalize_install_result + from ...models.results import InstallResult + + rebuild_result = finalize_install_result( + InstallResult(diagnostics=reintegration_diagnostics), force=False + ) + if reintegration_errors or rebuild_result.exit_code: + raise AggregateIntegrationError("rebuild", aggregate_rebuilds) return IntegrationCleanupOutcome( counts=counts, deployed_files=package_deployed_files, failed_paths=failed_hook_paths, error_count=result.get("errors", 0), + removed_paths=frozenset(removed_aggregate_paths), ) diff --git a/src/apm_cli/commands/uninstall/lockfile_state.py b/src/apm_cli/commands/uninstall/lockfile_state.py index fd79fe5c0e..b7e365b68c 100644 --- a/src/apm_cli/commands/uninstall/lockfile_state.py +++ b/src/apm_cli/commands/uninstall/lockfile_state.py @@ -11,6 +11,7 @@ def reconcile_uninstall_deployment_state( deploy_root: Path, all_deployed_files: set[str], surviving_deployed_files: dict[str, set[str]], + removed_paths: frozenset[str] = frozenset(), fully_refreshed_dependency_keys: set[str] | None = None, ) -> bool: """Reconcile survivor ownership and hashes without persisting.""" @@ -31,6 +32,20 @@ def reconcile_uninstall_deployment_state( for record in previous_ledger.records.values(): records_by_value.setdefault(record.locator.value, []).append(record) materializations: list[MaterializationResult] = [] + # Only the cleanup owner can attest removal. Empty/failed integration or + # filesystem absence alone is not proof. Current successful replacements + # supersede these removals in DeploymentReconciler. + for path in sorted(removed_paths): + for prior in records_by_value.get(path, ()): + materializations.append( + MaterializationResult( + locator=prior.locator, + owners=frozenset(prior.owners), + status=MaterializationStatus.REMOVED, + content_hash=None, + validation=NativePayloadValidation(valid=True, contract="uninstall-cleanup"), + ) + ) for dep_key, deployed_files in surviving_deployed_files.items(): transferred = sorted(all_deployed_files.intersection(deployed_files)) hashes = compute_deployed_hashes(transferred, deploy_root) diff --git a/src/apm_cli/core/deployment_ledger.py b/src/apm_cli/core/deployment_ledger.py index 9f6d00d139..b74292ebd1 100644 --- a/src/apm_cli/core/deployment_ledger.py +++ b/src/apm_cli/core/deployment_ledger.py @@ -115,6 +115,8 @@ def cleanup_snapshot( if owner in selected for path in dependency.deployed_files } + if "." in selected: + paths.update(lockfile.local_deployed_files) hashes: dict[str, str] = {} for record in DeploymentLedgerCodec.from_lockfile(lockfile).records.values(): if ( @@ -128,6 +130,9 @@ def cleanup_snapshot( continue for path, content_hash in dependency.deployed_file_hashes.items(): hashes.setdefault(path, content_hash) + if "." in selected: + for path, content_hash in lockfile.local_deployed_file_hashes.items(): + hashes.setdefault(path, content_hash) return DeploymentCleanupSnapshot(paths=frozenset(paths), hashes=hashes) @staticmethod @@ -298,6 +303,17 @@ def apply_to_lockfile( dependency.deployed_file_hashes = dict(sorted(dependency_hashes[owner].items())) lockfile.local_deployed_files = sorted(dict.fromkeys(local_files)) lockfile.local_deployed_file_hashes = dict(sorted(local_hashes.items())) + # LockFile.read synthesizes "." from the local compatibility view. + # Keep that existing virtual view in step with the authoritative + # projection; it must not keep an otherwise empty lockfile alive. + if "." in lockfile.dependencies: + if local_files: + lockfile.dependencies["."].deployed_files = list(lockfile.local_deployed_files) + lockfile.dependencies["."].deployed_file_hashes = dict( + lockfile.local_deployed_file_hashes + ) + else: + del lockfile.dependencies["."] lockfile.mcp_target_servers = { runtime: sorted(dict.fromkeys(servers)) for runtime, servers in sorted(service_targets["mcp"].items()) diff --git a/src/apm_cli/core/deployment_state.py b/src/apm_cli/core/deployment_state.py index 38e3002091..862b5f3c0e 100644 --- a/src/apm_cli/core/deployment_state.py +++ b/src/apm_cli/core/deployment_state.py @@ -168,6 +168,7 @@ def reconcile_package_claims( current_claims: Mapping[str, Iterable[str]], prior_files: Mapping[str, Iterable[str]], prior_hashes: Mapping[str, Mapping[str, str]], + shared_paths: frozenset[str] = frozenset(), ) -> dict[str, PackageDeploymentClaims]: """Resolve legacy last-writer claims and prior carry-forward eligibility.""" normalized_current = {owner: tuple(paths) for owner, paths in current_claims.items()} @@ -179,10 +180,14 @@ def reconcile_package_claims( claims: dict[str, PackageDeploymentClaims] = {} for owner in package_keys: current = tuple( - path for path in normalized_current.get(owner, ()) if last_owner[path] == owner + path + for path in normalized_current.get(owner, ()) + if path in shared_paths or last_owner[path] == owner ) eligible_prior = tuple( - path for path in prior_files.get(owner, ()) if last_owner.get(path, owner) == owner + path + for path in prior_files.get(owner, ()) + if path in shared_paths or last_owner.get(path, owner) == owner ) eligible_prior_set = set(eligible_prior) claims[owner] = PackageDeploymentClaims( @@ -196,6 +201,30 @@ def reconcile_package_claims( ) return claims + @staticmethod + def merge_aggregate_records( + batches: Iterable[Mapping[str, DeploymentRecord]], + shared_paths: frozenset[str], + ) -> dict[str, DeploymentRecord]: + """Collect ordered aggregate owners, freezing each final record only once.""" + records: dict[str, DeploymentRecord] = {} + aggregate_owners: dict[str, dict[str, None]] = {} + for batch in batches: + for key, record in batch.items(): + if record.locator.value in shared_paths: + owners = aggregate_owners.setdefault(key, {}) + owners.update(dict.fromkeys(record.owners)) + records[key] = record + for key, owners in aggregate_owners.items(): + record = records[key] + records[key] = DeploymentRecord( + locator=record.locator, + owners=tuple(owners), + active_owner=record.active_owner, + content_hash=record.content_hash, + ) + return records + def reconcile( self, previous: DeploymentLedger, diff --git a/src/apm_cli/install/helpers/security_scan.py b/src/apm_cli/install/helpers/security_scan.py index d0623feb47..241fa33ee8 100644 --- a/src/apm_cli/install/helpers/security_scan.py +++ b/src/apm_cli/install/helpers/security_scan.py @@ -16,6 +16,7 @@ def _pre_deploy_security_scan( package_name: str = "", force: bool = False, logger=None, + show_force_hint: bool = True, ) -> bool: """Scan authorized deployable source files for hidden characters before deployment. @@ -50,10 +51,11 @@ def _pre_deploy_security_scan( logger.error_detail( " |-- Fix the reported file(s) in the package source, then reinstall" ) - logger.error_detail( - " |-- Use --force only after reviewing the reported findings " - f"from {printable_ascii_text(str(source_plan.source_root))}" - ) + if show_force_hint: + logger.error_detail( + " |-- Use --force only after reviewing the reported findings " + f"from {printable_ascii_text(str(source_plan.source_root))}" + ) return False return True diff --git a/src/apm_cli/install/manifest_reconcile.py b/src/apm_cli/install/manifest_reconcile.py index 4cefc0cbfe..47dc799372 100644 --- a/src/apm_cli/install/manifest_reconcile.py +++ b/src/apm_cli/install/manifest_reconcile.py @@ -706,6 +706,7 @@ def reconcile_target_deployed_files( if dep_key not in (dependency_keys or set()) for path in dependency.deployed_files } + survivor_files.update(lockfile.local_deployed_files) desired_owners = ( DeploymentLedgerCodec.valid_owner_keys( lockfile, diff --git a/src/apm_cli/install/phases/lockfile.py b/src/apm_cli/install/phases/lockfile.py index d0b6a0d376..fb189f4da2 100644 --- a/src/apm_cli/install/phases/lockfile.py +++ b/src/apm_cli/install/phases/lockfile.py @@ -232,12 +232,22 @@ def _attach_deployed_files(self, lockfile: LockFile) -> None: previous.deployed_file_hashes if previous is not None else {} ) from apm_cli.core.deployment_state import DeploymentReconciler - + from apm_cli.integration.instruction_integrator import InstructionIntegrator + + shared_paths = InstructionIntegrator.aggregate_paths(self.ctx.targets) + current_claims = dict(self.ctx.package_deployed_files) + local_aggregates = shared_paths.intersection(getattr(self.ctx, "local_deployed_files", ())) + if local_aggregates: + current_claims["."] = sorted(local_aggregates) + package_keys = list(lockfile.dependencies) + if local_aggregates and "." not in package_keys: + package_keys.append(".") package_claims = DeploymentReconciler.reconcile_package_claims( - package_keys=lockfile.dependencies, - current_claims=self.ctx.package_deployed_files, + package_keys=package_keys, + current_claims=current_claims, prior_files=prior_files_by_package, prior_hashes=prior_hashes_by_package, + shared_paths=shared_paths, ) declared = declared_target_profiles(self.ctx) diagnostics = getattr(self.ctx, "diagnostics", None) @@ -255,16 +265,30 @@ def _attach_deployed_files(self, lockfile: LockFile) -> None: # not removed, so a "Repaired inactive-target entries" report would be # misleading. lockfile_only = getattr(self.ctx, "lockfile_only", False) - canonical_records = {} + canonical_batches = [] + aggregate_hashes: dict[str, str | None] = {} + + def _invalidate_aggregate_hashes(cleanup) -> None: + for path in (*cleanup.deleted, *cleanup.retained): + aggregate_hashes.pop(path, None) + ghost_count = 0 - for dep_key in lockfile.dependencies: + for dep_key in package_keys: claim = package_claims[dep_key] current = list(claim.current_files) retained_hashes = cleanup_retained.get(dep_key, {}) + hashable = [path for path in current if path not in retained_hashes] current_hashes = compute_deployed_hashes( - (path for path in current if path not in retained_hashes), + (path for path in hashable if path not in shared_paths), self.ctx.project_root, ) + for path in shared_paths.intersection(hashable): + if path not in aggregate_hashes: + aggregate_hashes[path] = compute_deployed_hashes( + [path], self.ctx.project_root + ).get(path) + if aggregate_hashes[path] is not None: + current_hashes[path] = aggregate_hashes[path] prior_files = list(claim.prior_files) prior_hashes = claim.prior_hashes @@ -289,6 +313,7 @@ def _log_ghost_drop(path: str, package_key: str = dep_key) -> None: declared_targets=declared, diagnostics=diagnostics, on_ghost_drop=None if lockfile_only else _log_ghost_drop, + on_cleanup=_invalidate_aggregate_hashes, prior_ledger=prior_ledger, cleanup_retained_hashes=retained_hashes, current_run_trusted=diagnostics.count_for_package(dep_key, "error") == 0, @@ -302,9 +327,13 @@ def _log_ghost_drop(path: str, package_key: str = dep_key) -> None: # leave deployed_files untouched so the whole-dep # _merge_existing path can preserve it intact. continue - canonical_records.update(ledger.records) + canonical_batches.append(ledger.records) + if canonical_batches: from apm_cli.core.deployment_ledger import DeploymentLedgerCodec + canonical_records = DeploymentReconciler.merge_aggregate_records( + canonical_batches, shared_paths + ) DeploymentLedgerCodec.apply_to_lockfile( DeploymentLedger(records=canonical_records), lockfile, @@ -493,17 +522,32 @@ def _preserve_existing_local_state(self, lockfile: LockFile) -> None: """Keep local fields until post_deps_local reconciles content hashes.""" if self.ctx.existing_lockfile: from apm_cli.core.deployment_ledger import DeploymentLedgerCodec - - DeploymentLedgerCodec.replace_legacy_owner( - lockfile, - ".", - list(self.ctx.existing_lockfile.local_deployed_files), - copy.deepcopy(self.ctx.existing_lockfile.local_deployed_file_hashes), + from apm_cli.integration.instruction_integrator import InstructionIntegrator + + # Attachment already proved the completed aggregate. Unlike other + # local outputs, USER aggregate hashes have no later local persist + # pass; never replace their current proof with a stale root hash. + current_aggregates = InstructionIntegrator.aggregate_paths( + self.ctx.targets + ).intersection(lockfile.local_deployed_files) + local_files = sorted( + set(self.ctx.existing_lockfile.local_deployed_files) | current_aggregates ) + local_hashes = copy.deepcopy(self.ctx.existing_lockfile.local_deployed_file_hashes) + for path in current_aggregates: + local_hashes.pop(path, None) + if path in lockfile.local_deployed_file_hashes: + local_hashes[path] = lockfile.local_deployed_file_hashes[path] if "." in self.ctx.existing_lockfile.dependencies: lockfile.dependencies["."] = copy.deepcopy( self.ctx.existing_lockfile.dependencies["."] ) + DeploymentLedgerCodec.replace_legacy_owner( + lockfile, + ".", + local_files, + local_hashes, + ) if self.ctx.logger: self.ctx.logger.verbose_detail( "Carrying forward local .apm state pending hash reconciliation: " diff --git a/src/apm_cli/install/services.py b/src/apm_cli/install/services.py index 5cf7a726c1..d02de1c38b 100644 --- a/src/apm_cli/install/services.py +++ b/src/apm_cli/install/services.py @@ -225,6 +225,7 @@ def integrate_package_primitives( # noqa: PLR0913 dep_target_subset: list[str] | None = None, trust_bin: bool | None = None, bin_skip_reason_override: str | None = None, + show_force_hint: bool = True, ) -> dict: """Run the full integration pipeline for a single package. @@ -365,6 +366,7 @@ def _reconcile_excluded_targets() -> None: package_name=package_name, force=force, logger=logger, + show_force_hint=show_force_hint, ): return result @@ -689,7 +691,7 @@ def _log_integration(msg): return result -def integrate_local_content( +def integrate_local_content( # noqa: PLR0913 -- explicit integration collaborator facade project_root: Path, *, targets: Any, @@ -706,6 +708,7 @@ def integrate_local_content( scope: InstallScope | None = None, source_root: Path | None = None, ctx: InstallContext | None = None, + show_force_hint: bool = True, ) -> dict: """Integrate primitives from the project's own .apm/ directory. @@ -769,6 +772,7 @@ def integrate_local_content( scope=scope, ctx=ctx, is_first_party=True, + show_force_hint=show_force_hint, ) diff --git a/src/apm_cli/integration/instruction_integrator.py b/src/apm_cli/integration/instruction_integrator.py index 497f7ad38f..adb5f24dbc 100644 --- a/src/apm_cli/integration/instruction_integrator.py +++ b/src/apm_cli/integration/instruction_integrator.py @@ -29,6 +29,7 @@ if TYPE_CHECKING: from apm_cli.install.deployable_source_plan import DeployableSourcePlan from apm_cli.integration.targets import TargetProfile + from apm_cli.utils.diagnostics import DiagnosticCollector @dataclass(frozen=True) @@ -494,16 +495,35 @@ def sync_for_target( apm_package, project_root: Path, managed_files: set[str] | None = None, - ) -> dict[str, int]: + *, + managed_file_hashes: dict[str, str] | None = None, + diagnostics: DiagnosticCollector | None = None, + ) -> dict[str, int | tuple[str, ...]]: """Remove APM-managed instruction files for a single *target*.""" mapping = target.primitives.get("instructions") if not mapping: return {"files_removed": 0, "errors": 0} effective_root = mapping.deploy_root or target.root_dir if mapping.format_id == "copilot_user_instructions": - prefix = f"{effective_root}/copilot-instructions.md" - legacy_pattern = None - legacy_dir = None + from apm_cli.integration.cleanup import remove_stale_deployed_files + from apm_cli.utils.diagnostics import DiagnosticCollector + + # Generated aggregates are whole-file rebuilds, but only after + # the cleanup owner's containment and recorded-hash checks pass. + cleanup = remove_stale_deployed_files( + project_root=project_root, + stale_paths=self.aggregate_paths([target]).intersection(managed_files or ()), + recorded_hashes=managed_file_hashes or {}, + diagnostics=diagnostics or DiagnosticCollector(), + dep_key="", + targets=[target], + user_scope=True, + ) + return { + "files_removed": len(cleanup.deleted), + "errors": len(cleanup.retained), + "removed_paths": tuple(cleanup.deleted), + } else: prefix = f"{effective_root}/{mapping.subdir}/" legacy_dir = project_root / effective_root / mapping.subdir @@ -535,6 +555,16 @@ def sync_for_target( # Copilot user-scope concat support # ------------------------------------------------------------------ + @staticmethod + def aggregate_paths(targets: list[TargetProfile]) -> frozenset[str]: + """Return generated instruction paths whose contributions share ownership.""" + return frozenset( + (Path(mapping.deploy_root or target.root_dir) / "copilot-instructions.md").as_posix() + for target in targets + if (mapping := target.primitives.get("instructions")) is not None + and mapping.format_id == "copilot_user_instructions" + ) + # Sentinel line written as the first line of every APM-managed # copilot-instructions.md. Its presence distinguishes the file from # a user-authored one, enabling multi-package accumulation without diff --git a/tests/integration/test_architecture_copilot_aggregate.py b/tests/integration/test_architecture_copilot_aggregate.py new file mode 100644 index 0000000000..24ba18851a --- /dev/null +++ b/tests/integration/test_architecture_copilot_aggregate.py @@ -0,0 +1,78 @@ +"""Executable boundary checks for generated-aggregate lifecycle routing.""" + +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] +RULE = "install-deployment-copilot-aggregate" + + +def test_copilot_aggregate_routes_are_registered_and_satisfied() -> None: + """The stable architecture entrypoint accepts the production routes.""" + report = run_selected_rules(ROOT, (RULE,)) + assert report.failures == () + assert report.violations == () + + +@pytest.mark.parametrize( + ("path", "old", "new"), + [ + ( + "src/apm_cli/install/phases/lockfile.py", + "DeploymentReconciler.merge_aggregate_records(", + "parallel_merge(", + ), + ( + "src/apm_cli/commands/uninstall/engine.py", + "root_result = integrate_local_content(", + "root_result = unscanned_local_content(", + ), + ( + "src/apm_cli/integration/instruction_integrator.py", + "cleanup = remove_stale_deployed_files(", + "cleanup = unsafe_delete(", + ), + ( + "src/apm_cli/models/apm_package.py", + "package.source = dep_ref.to_github_url()", + "package.source = None", + ), + ( + "src/apm_cli/commands/uninstall/engine.py", + "rebuild_result = finalize_install_result(", + "rebuild_result = parallel_outcome(", + ), + ( + "src/apm_cli/commands/uninstall/cli.py", + "removed_paths=(", + "discarded_removed_paths=(", + ), + ( + "src/apm_cli/commands/uninstall/engine.py", + "removed_paths=frozenset(removed_aggregate_paths)", + "removed_paths=frozenset()", + ), + ( + "src/apm_cli/commands/uninstall/lockfile_state.py", + "status=MaterializationStatus.REMOVED", + "status=MaterializationStatus.SKIPPED", + ), + ( + "src/apm_cli/install/phases/lockfile.py", + "current_aggregates = InstructionIntegrator.aggregate_paths(", + "current_aggregates = parallel_aggregate_paths(", + ), + ], +) +def test_copilot_aggregate_guard_rejects_bypassed_owner(path: str, old: str, new: str) -> None: + """Each centralization stays guarded against a sibling or missing authority.""" + source = (ROOT / path).read_text(encoding="utf-8") + assert source.count(old) == 1 + report = run_selected_rules(ROOT, (RULE,), source_overrides={path: source.replace(old, new)}) + assert report.failures == () + assert any(violation.rule_id == RULE for violation in report.violations) diff --git a/tests/integration/test_architecture_owner_rule_mutations.py b/tests/integration/test_architecture_owner_rule_mutations.py index 036f70b41d..121b45725b 100644 --- a/tests/integration/test_architecture_owner_rule_mutations.py +++ b/tests/integration/test_architecture_owner_rule_mutations.py @@ -317,6 +317,14 @@ class MutationCase: new='if target.name == "copilot" and mapping is not None:', intent="Local bundle routing branches on target names instead of target primitives.", ), + MutationCase( + guard_id="install-deployment-copilot-aggregate", + rule_id="install-deployment-copilot-aggregate", + path="src/apm_cli/install/phases/lockfile.py", + old="DeploymentReconciler.merge_aggregate_records(", + new="parallel_aggregate_merge(", + intent="Aggregate ledger accumulation bypasses the canonical ownership reconciler.", + ), MutationCase( guard_id="install-deployment-executable-trust-context", rule_id="install-deployment-executable-trust-context", diff --git a/tests/integration/test_copilot_aggregate_uninstall.py b/tests/integration/test_copilot_aggregate_uninstall.py new file mode 100644 index 0000000000..27f7f6b062 --- /dev/null +++ b/tests/integration/test_copilot_aggregate_uninstall.py @@ -0,0 +1,351 @@ +"""Source-backed, installed-CLI proofs for the generated Copilot aggregate.""" + +from __future__ import annotations + +import hashlib +import os +import re +from pathlib import Path +from urllib.parse import urlparse + +import pytest + +from apm_cli.deps.lockfile import LockFile +from apm_cli.models.dependency.selection import parse_dependency_entry +from apm_cli.utils.yaml_io import dump_yaml, load_yaml +from tests.utils.apm_lifecycle_runner import ApmLifecycleRunner, CommandResult +from tests.utils.isolated_apm_environment import IsolatedApmEnvironment +from tests.utils.local_git_repository import LocalGitRepositoryFactory +from tests.utils.local_package import LocalPackageFactory + +pytestmark = [ + pytest.mark.e2e, + pytest.mark.requires_apm_binary, + pytest.mark.lifecycle_smoke, + pytest.mark.lifecycle_merge_group, +] + +_HEADER = "" +_NOTES = b"# Independent notes\nDo not attribute or modify these user bytes.\n" +_ROOT_BODY = "# Root contribution\nKeep the actual root-local instruction.\n" + + +def _assert_recovered_sections( + aggregate: Path, survivor_source: str, survivor_body: str, removed_body: str +) -> None: + """Validate exact source sections without imposing root/package ordering.""" + recovered = aggregate.read_text() + assert removed_body.strip() not in recovered + identities = re.findall(r"", recovered) + assert sorted(urlparse(identity) for identity in identities) == sorted( + [urlparse(survivor_source), urlparse("local")] + ) + sections = re.findall( + r"\n(.*?)\n", recovered, re.S + ) + assert len(sections) == 2 + assert {urlparse(source): body for source, body in sections} == { + urlparse(survivor_source): survivor_body.strip(), + urlparse("local"): _ROOT_BODY.strip(), + } + assert recovered.count(survivor_body.strip()) == 1 + assert recovered.count(_ROOT_BODY.strip()) == 1 + + +@pytest.mark.parametrize( + "obligation", + [ + "initial-owners", + "removed-section", + "survivor-identity", + "survivor-state", + "collision", + "root", + "edited", + "unsafe-survivor", + "survivor-empty", + "survivor-missing", + "survivor-ineligible", + "root-missing", + ], +) +def test_generated_copilot_aggregate_lifecycle( + tmp_path: Path, apm_binary_path: Path, obligation: str +) -> None: + """Keep owner, cleanup and identity failures independent, never xfailed.""" + isolated = IsolatedApmEnvironment.create(tmp_path / "scenario", base_env=os.environ) + factory = LocalPackageFactory(isolated.package_root) + repositories = LocalGitRepositoryFactory( + isolated.repository_root, env=isolated.subprocess_env() + ) + declarations = [] + sources = {} + refs = {} + commits = {} + bodies = {} + rewrites = [] + for name in ("primary", "survivor"): + package = factory.create(name, targets=["copilot"]) + bodies[name] = f"# {name.title()} instructions\nAuthored {name} contribution only.\n" + instruction = factory.add_instruction( + package, name, "---\napplyTo: '**'\n---\n\n" + bodies[name] + ) + repository = repositories.create(name, source_tree=package.root) + commit = repositories.commit(repository, message=f"Author {name} instructions") + remote = f"https://gitlab.com/apm-aggregate-fixture/{name}.git" + declaration = {"git": remote, "type": "gitlab", "ref": commit.sha, "alias": name} + declarations.append(declaration) + refs[name] = parse_dependency_entry(declaration) + commits[name] = commit.sha + sources[name] = { + "apm.yml": package.manifest_path.read_bytes(), + f".apm/instructions/{name}.instructions.md": instruction.read_bytes(), + } + rewrites.append((repository, remote)) + environment = repositories.url_rewrite_subprocess_env_many(rewrites) + consumer = LocalPackageFactory(isolated.work_root).create("consumer") + manifest = { + "name": "consumer", + "version": "0.1.0", + "targets": ["copilot"], + "dependencies": {"apm": declarations}, + } + dump_yaml(manifest, isolated.config_root / "apm.yml") + has_root = obligation in {"root", "root-missing", "edited", "unsafe-survivor"} + if has_root: + root_instruction = isolated.home / ".apm/instructions/root.instructions.md" + root_instruction.parent.mkdir(parents=True) + root_instruction.write_text(_ROOT_BODY, encoding="utf-8") + + aggregate = isolated.home / ".copilot/copilot-instructions.md" + notes = aggregate.with_name("lifecycle-user-notes.md") + notes.parent.mkdir() + notes.write_bytes(_NOTES) + assert not aggregate.exists(), "Supported generated input must start absent" + collision = b"# User instructions\nNever adopt this headerless content.\n" + if obligation == "collision": + aggregate.write_bytes(collision) + runner = ApmLifecycleRunner([str(apm_binary_path)]) + project_manifest = consumer.manifest_path.read_bytes() + project_notes = consumer.root / "unowned-notes.md" + project_notes.write_bytes(_NOTES) + + def command(*args: str, expected_returncode: int = 0) -> CommandResult: + result = runner.run(args, scenario_id=obligation, cwd=consumer.root, env=environment) + print("COMMAND", result.command, "EXIT", result.returncode) + print(result.stdout, result.stderr) + assert result.returncode == expected_returncode, (result.stdout, result.stderr) + assert notes.read_bytes() == _NOTES, "Unowned neighboring notes changed" + assert consumer.manifest_path.read_bytes() == project_manifest + assert project_notes.read_bytes() == _NOTES + assert not (consumer.root / "apm_modules").exists() + return result + + command( + "install", "--global", "--target", "copilot", "--no-policy", "--parallel-downloads", "0" + ) + lock = LockFile.read(isolated.config_root / "apm.lock.yaml") + assert lock is not None + expected_keys = {refs[name].get_unique_key() for name in refs} + assert {dep.get_unique_key() for dep in lock.get_package_dependencies()} == expected_keys + for name, ref in refs.items(): + locked = lock.dependencies[ref.get_unique_key()] + assert locked.resolved_commit == commits[name] + assert locked.to_dependency_ref().get_unique_key() == ref.get_unique_key() + for relative, content in sources[name].items(): + assert ( + ref.get_install_path(isolated.config_root / "apm_modules") / relative + ).read_bytes() == content + assert load_yaml(isolated.config_root / "apm.yml") == manifest + initial = aggregate.read_text(encoding="utf-8") + print("INITIAL AGGREGATE", initial) + print("INITIAL LEDGER", lock.deployment_ledger) + if obligation == "collision": + assert aggregate.read_bytes() == collision + assert not lock.deployment_ledger.records, "Unmanaged output acquired an ownership claim" + return + assert initial.startswith(_HEADER) + records = list(lock.deployment_ledger.records.values()) + assert len(records) == 1 + assert set(records[0].owners) == expected_keys | ({"."} if has_root else set()) + for body in bodies.values(): + assert initial.count(body.strip()) == 1 + if obligation == "initial-owners": + records = list(lock.deployment_ledger.records.values()) + assert len(records) == 1 + assert set(records[0].owners) == expected_keys, "Initial aggregate lost an authored owner" + assert records[0].active_owner in expected_keys + return + if has_root: + assert initial.count(_ROOT_BODY.strip()) == 1 + aggregate_rel = ".copilot/copilot-instructions.md" + survivor_path = refs["survivor"].get_install_path(isolated.config_root / "apm_modules") + installed_instruction = survivor_path / ".apm/instructions/survivor.instructions.md" + + def assert_survivor_state() -> LockFile: + current_lock = LockFile.read(isolated.config_root / "apm.lock.yaml") + assert current_lock is not None + survivor = refs["survivor"] + assert set(current_lock.dependencies) == {survivor.get_unique_key()} | ( + {"."} if has_root else set() + ) + locked = current_lock.dependencies[survivor.get_unique_key()] + assert locked.name == "survivor" + assert locked.resolved_commit == commits["survivor"] + assert locked.to_dependency_ref().get_unique_key() == survivor.get_unique_key() + assert urlparse(locked.to_dependency_ref().to_github_url()) == urlparse( + survivor.to_github_url() + ) + assert ( + load_yaml(isolated.config_root / "apm.yml")["dependencies"]["apm"] == declarations[1:] + ) + assert not refs["primary"].get_install_path(isolated.config_root / "apm_modules").exists() + for relative, content in sources["survivor"].items(): + assert (survivor_path / relative).read_bytes() == content + return current_lock + + if obligation in {"survivor-empty", "survivor-missing", "survivor-ineligible"}: + relative = ".apm/instructions/survivor.instructions.md" + if obligation == "survivor-empty": + installed_instruction.write_bytes(b"") + sources["survivor"][relative] = b"" + elif obligation == "survivor-missing": + installed_instruction.unlink() + del sources["survivor"][relative] + else: + survivor_manifest = load_yaml(survivor_path / "apm.yml") + survivor_manifest["targets"] = ["claude"] + dump_yaml(survivor_manifest, survivor_path / "apm.yml") + sources["survivor"]["apm.yml"] = (survivor_path / "apm.yml").read_bytes() + command("uninstall", declarations[0]["git"], "--global") + assert not aggregate.exists() + current_lock = assert_survivor_state() + assert not current_lock.deployment_ledger.records, ( + "Removed aggregate retains canonical ownership" + ) + assert not current_lock.local_deployed_files + assert not current_lock.local_deployed_file_hashes + for dep in current_lock.dependencies.values(): + assert aggregate_rel not in dep.deployed_files + assert aggregate_rel not in dep.deployed_file_hashes + if obligation == "survivor-missing": + assert not installed_instruction.exists() + return + if obligation == "edited": + # Negative cleanup control, not a promise of editable generated regions. + edited = aggregate.read_bytes() + b"\nChanged after install; refuse hash mismatch.\n" + aggregate.write_bytes(edited) + result = command("uninstall", declarations[0]["git"], "--global", expected_returncode=1) + assert aggregate.read_bytes() == edited, "Hash-refused aggregate was overwritten" + elif obligation == "unsafe-survivor": + installed_instruction.write_text("# Unsafe\nHidden \u202e instruction.\n", encoding="utf-8") + result = command("uninstall", declarations[0]["git"], "--global", expected_returncode=1) + assert bodies["survivor"].strip() not in aggregate.read_text(), ( + "Rejected source was deployed" + ) + if obligation in {"edited", "unsafe-survivor"}: + failed_lock = LockFile.read(isolated.config_root / "apm.lock.yaml") + assert failed_lock is not None + assert {dep.get_unique_key() for dep in failed_lock.get_package_dependencies()} == { + refs["survivor"].get_unique_key() + } + assert ( + load_yaml(isolated.config_root / "apm.yml")["dependencies"]["apm"] == declarations[1:] + ) + assert not refs["primary"].get_install_path(isolated.config_root / "apm_modules").exists() + failed_records = list(failed_lock.deployment_ledger.records.values()) + assert len(failed_records) == 1, "Failure erased retained aggregate ownership" + assert failed_records[0].content_hash == records[0].content_hash + output = " ".join((result.stdout + result.stderr).split()) + assert ( + "aggregate cleanup" in output.lower() + if obligation == "edited" + else "aggregate rebuild" in output.lower() + ) + assert aggregate_rel in output + assert "package removal" in output.lower() and "completed" in output.lower() + assert "apm install --global" in output + assert "Uninstall complete:" not in output + assert "--force" not in output + if obligation == "edited": + assert "back up" in output.lower() and "move" in output.lower() + backup = aggregate.with_name("copilot-instructions.md.review-backup") + aggregate.rename(backup) + assert backup.read_bytes() == edited + else: + assert "source" in output.lower() + installed_instruction.write_bytes( + sources["survivor"][".apm/instructions/survivor.instructions.md"] + ) + # An authorized root may already have rebuilt while the survivor was blocked. + # Execute the displayed scope-correct action from an unrelated project. + command("install", "--global", "--no-policy", "--parallel-downloads", "0") + current_lock = assert_survivor_state() + _assert_recovered_sections( + aggregate, refs["survivor"].to_github_url(), bodies["survivor"], bodies["primary"] + ) + assert root_instruction.read_text() == _ROOT_BODY + records = list(current_lock.deployment_ledger.records.values()) + assert len(records) == 1 + assert set(records[0].owners) == {refs["survivor"].get_unique_key(), "."} + assert ( + records[0].content_hash + == "sha256:" + hashlib.sha256(aggregate.read_bytes()).hexdigest() + ) + if obligation == "edited": + assert backup.read_bytes() == edited + return + + command("uninstall", declarations[0]["git"], "--global") + if obligation == "root-missing": + assert_survivor_state() + root_instruction.unlink() + command("uninstall", declarations[1]["git"], "--global") + assert not aggregate.exists() + assert load_yaml(isolated.config_root / "apm.yml")["dependencies"]["apm"] == [] + assert not survivor_path.exists() + assert not (isolated.config_root / "apm.lock.yaml").exists(), ( + "Empty lockfile retained stale root ownership" + ) + return + remaining = aggregate.read_text(encoding="utf-8") + print("POST-UNINSTALL AGGREGATE", remaining) + if obligation == "removed-section": + assert bodies["primary"].strip() not in remaining, "Removed-owner body survived" + assert "primary" not in remaining, "Removed-owner provenance survived" + elif obligation == "survivor-identity": + identities = re.findall(r"", remaining) + assert [urlparse(identity) for identity in identities] == [ + urlparse(refs["survivor"].to_github_url()) + ], "Survivor identity must be resolved and unique" + assert "apm:source:unknown" not in remaining + elif obligation == "root": + assert remaining.count(_ROOT_BODY.strip()) == 1, "Actual root-local contribution lost" + assert root_instruction.read_text(encoding="utf-8") == _ROOT_BODY + command("uninstall", declarations[1]["git"], "--global") + assert aggregate.read_text(encoding="utf-8") == ( + f"{_HEADER}\n\n{_ROOT_BODY.strip()}\n\n" + ) + root_lock = LockFile.read(isolated.config_root / "apm.lock.yaml") + assert root_lock is not None + assert root_lock.get_package_dependencies() == [] + assert [record.owners for record in root_lock.deployment_ledger.records.values()] == [ + (".",) + ] + else: + survivor = refs["survivor"] + lock = assert_survivor_state() + assert remaining.count(bodies["survivor"].strip()) == 1, "Survivor native content changed" + assert remaining == ( + f"{_HEADER}\n\n" + f"{bodies['survivor'].strip()}\n\n" + ) + records = list(lock.deployment_ledger.records.values()) + assert len(records) == 1 + assert records[0].owners == (survivor.get_unique_key(),) + assert records[0].active_owner == survivor.get_unique_key() + assert ( + records[0].content_hash + == "sha256:" + hashlib.sha256(aggregate.read_bytes()).hexdigest() + ) + assert notes.read_bytes() == _NOTES diff --git a/tests/unit/install/phases/test_lockfile_cross_package_reconcile.py b/tests/unit/install/phases/test_lockfile_cross_package_reconcile.py index 559b9d3731..0207966466 100644 --- a/tests/unit/install/phases/test_lockfile_cross_package_reconcile.py +++ b/tests/unit/install/phases/test_lockfile_cross_package_reconcile.py @@ -16,11 +16,127 @@ from types import SimpleNamespace +import pytest + from apm_cli.core.deployment_state import DeploymentReconciler from apm_cli.deps.lockfile import LockedDependency, LockFile from apm_cli.install.phases.lockfile import LockfileBuilder +@pytest.mark.parametrize("obligation", ["hash", "projection", "owner-work"]) +def test_aggregate_attachment_work_is_linear(tmp_path, monkeypatch, obligation) -> None: + """Count attachment work, not latency or the separate concatenation algorithm.""" + from apm_cli.core import deployment_state + from apm_cli.core.deployment_ledger import DeploymentLedgerCodec + from apm_cli.core.scope import InstallScope + from apm_cli.install.phases import lockfile as attachment + from apm_cli.integration.targets import resolve_targets + + original_hash = attachment.compute_file_hash + original_record = deployment_state.DeploymentRecord + original_apply = DeploymentLedgerCodec.apply_to_lockfile + counts = {} + + def hash_file(path): + counts["hash"] += 1 + counts["bytes"] += path.stat().st_size + return original_hash(path) + + def record(**kwargs): + counts["owners"] += len(kwargs["owners"]) + return original_record(**kwargs) + + def project(ledger, lockfile): + counts["projection"] += 1 + return original_apply(ledger, lockfile) + + monkeypatch.setattr(attachment, "compute_file_hash", hash_file) + monkeypatch.setattr(deployment_state, "DeploymentRecord", record) + monkeypatch.setattr(DeploymentLedgerCodec, "apply_to_lockfile", project) + observations = [] + for size in (50, 500): + counts.update(hash=0, bytes=0, owners=0, projection=0) + root = tmp_path / str(size) + path = root / ".copilot/copilot-instructions.md" + path.parent.mkdir(parents=True) + path.write_bytes(b"fixed-width-contribution\n" * size) + lock = LockFile() + claims = {} + for i in range(size): + key = f"fixture/package-{i}" + lock.add_dependency(LockedDependency(repo_url=key)) + claims[key] = [".copilot/copilot-instructions.md"] + ctx = _ctx( + package_deployed_files=claims, + targets=resolve_targets(root, user_scope=True, explicit_target=["copilot"]), + project_root=root, + ) + ctx.scope = InstallScope.USER + LockfileBuilder(ctx)._attach_deployed_files(lock) + observations.append(dict(counts)) + records = list(lock.deployment_ledger.records.values()) + assert len(records) == 1 + assert records[0].owners == tuple(claims) + assert records[0].active_owner == list(claims)[-1] + assert records[0].content_hash == original_hash(path) + assert all( + dep.deployed_files == [".copilot/copilot-instructions.md"] + for dep in lock.dependencies.values() + ) + small, large = observations + print("ATTACHMENT WORK", observations) + if obligation == "hash": + assert small["hash"] == large["hash"] == 1 + assert large["bytes"] == 10 * small["bytes"] + elif obligation == "projection": + assert small["projection"] == large["projection"] == 1 + else: + assert large["owners"] < 15 * small["owners"] + + +def test_aggregate_attachment_invalidates_digest_after_cleanup(tmp_path, monkeypatch): + """A reconciliation cleanup handoff invalidates this phase's digest, not future commands.""" + from apm_cli.core.scope import InstallScope + from apm_cli.install import manifest_reconcile + from apm_cli.install.phases.lockfile import compute_deployed_hashes + from apm_cli.integration.cleanup import CleanupResult + from apm_cli.integration.targets import resolve_targets + + relative = ".copilot/copilot-instructions.md" + target = tmp_path / relative + target.parent.mkdir() + target.write_text("before cleanup\n") + original = manifest_reconcile.reconcile_deployed_block + calls = [] + + def reconcile(**kwargs): + result = original(**kwargs) + calls.append(kwargs) + if len(calls) == 1: + # Simulate the owner's cleanup callback, then a replacement. + # This fixture does not claim a native cleanup execution. + target.write_text("replacement following cleanup\n") + if kwargs.get("on_cleanup") is not None: + kwargs["on_cleanup"](CleanupResult(deleted=[relative])) + return result + + monkeypatch.setattr(manifest_reconcile, "reconcile_deployed_block", reconcile) + lock = LockFile() + for name in ("first", "second"): + lock.add_dependency(LockedDependency(repo_url=f"fixture/{name}")) + ctx = _ctx( + package_deployed_files={key: [relative] for key in lock.dependencies}, + targets=resolve_targets(tmp_path, user_scope=True, explicit_target=["copilot"]), + project_root=tmp_path, + ) + ctx.scope = InstallScope.USER + LockfileBuilder(ctx)._attach_deployed_files(lock) + assert ( + next(iter(lock.deployment_ledger.records.values())).content_hash + == compute_deployed_hashes([relative], tmp_path)[relative] + ) + + def _target(name, root_dir=".claude"): return SimpleNamespace(name=name, root_dir=root_dir, primitives={}) diff --git a/tests/unit/install/test_install_copilot_user_instructions.py b/tests/unit/install/test_install_copilot_user_instructions.py index 21ce876998..13284fbfc0 100644 --- a/tests/unit/install/test_install_copilot_user_instructions.py +++ b/tests/unit/install/test_install_copilot_user_instructions.py @@ -262,4 +262,4 @@ def test_project_scope_unaffected(self): assert result.files_integrated == 1 deployed = result.target_paths[0] assert deployed.name == "coding.instructions.md" - assert ".copilot" not in str(deployed) + assert deployed.relative_to(project_root).parent == Path(".github/instructions") diff --git a/tests/unit/scripts/test_architecture_runner.py b/tests/unit/scripts/test_architecture_runner.py index 59f7b4ed61..e096d3bc1c 100644 --- a/tests/unit/scripts/test_architecture_runner.py +++ b/tests/unit/scripts/test_architecture_runner.py @@ -621,6 +621,7 @@ def exiting_import( install-deployment-bundle-native-layout install-deployment-base-integrator install-deployment-cached-claude-skill-metadata +install-deployment-copilot-aggregate install-deployment-dependency-winner-selection install-deployment-deployment-frame-projection install-deployment-executable-trust-context diff --git a/tests/unit/test_aggregate_installed_source.py b/tests/unit/test_aggregate_installed_source.py new file mode 100644 index 0000000000..8293456dc0 --- /dev/null +++ b/tests/unit/test_aggregate_installed_source.py @@ -0,0 +1,24 @@ +"""Installed source identity is distinct from acquisition trust.""" + +from urllib.parse import urlparse + +import pytest + +from apm_cli.models.apm_package import APMPackage, restore_installed_package_source +from apm_cli.models.dependency.reference import DependencyReference + +pytestmark = pytest.mark.unit + + +@pytest.mark.parametrize("source", [None, "git", "local", "registry"]) +def test_restore_installed_source_only_changes_git_identity(source: str | None) -> None: + """Local and registry identities must never become Git acquisition claims.""" + package = APMPackage(name="fixture", version="1.0.0", source="authored") + dependency = DependencyReference(repo_url="fixture/package", source=source) + + restore_installed_package_source(package, dependency) + + if source in (None, "git"): + assert urlparse(package.source) == urlparse(dependency.to_github_url()) + else: + assert package.source == "authored" diff --git a/tests/unit/test_uninstall_engine_helpers.py b/tests/unit/test_uninstall_engine_helpers.py index e1435b7d77..98c950bc94 100644 --- a/tests/unit/test_uninstall_engine_helpers.py +++ b/tests/unit/test_uninstall_engine_helpers.py @@ -46,6 +46,54 @@ def _make_logger(): return logger +@pytest.mark.parametrize("user_scope", [False, True], ids=["project", "user"]) +@pytest.mark.parametrize("phase", ["cleanup", "rebuild"]) +def test_aggregate_recovery_is_visible_sanitized_and_scope_correct(tmp_path, user_scope, phase): + """Default semantic output preserves safe failure facts and the selected scope.""" + from apm_cli.commands.uninstall.cli import _sync_integrations_for_manifest + from apm_cli.commands.uninstall.engine import AggregateIntegrationError + + logger = _make_logger() + error = AggregateIntegrationError( + phase, frozenset({".copilot/copilot-instructions.md\x1b[31m"}) + ) + error.__cause__ = ValueError("private-source-token") + with ( + patch("apm_cli.commands.uninstall.cli.APMPackage.from_apm_yml"), + patch( + "apm_cli.commands.uninstall.cli._sync_integrations_after_uninstall", side_effect=error + ), + ): + outcome, reported_error = _sync_integrations_for_manifest( + manifest_path=tmp_path / "apm.yml", + deploy_root=tmp_path, + all_deployed_files=set(), + logger=logger, + user_scope=user_scope, + lockfile=None, + modules_dir=tmp_path / "apm_modules", + deployed_file_hashes={}, + default_counts={}, + ) + messages = "\n".join(call.args[0] for call in logger.warning.call_args_list) + assert f"aggregate {phase}" in messages + assert ".copilot/copilot-instructions.md" in messages + assert "\x1b" not in messages and "private-source-token" not in messages + assert "removal completed" in messages + expected = "apm install --global" if user_scope else "apm install" + assert f"Run '{expected}'" in messages + if not user_scope: + assert "--global" not in messages + assert "--force" not in messages + if phase == "cleanup": + assert "Back up and move" in messages + else: + assert "source errors" in messages + assert reported_error is error + assert outcome.removed_paths == frozenset() + logger.success.assert_not_called() + + # =========================================================================== # _parse_dependency_entry # =========================================================================== From 55589b4f6a7dd760064eff07457f19dfa536cfec Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Wed, 9 Sep 2026 11:26:23 +0200 Subject: [PATCH 03/10] docs(install): explain aggregate reconstruction and recovery Preserve only aggregate user guidance from #2820, retaining current main documentation ordering and the active v0.1 assessment. Explain whole-file source authoring, truthful refusal receipts and global-scope recovery without a force bypass. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 4 ++++ .../content/docs/reference/targets-matrix.md | 19 +++++++++++++++++++ .../.apm/skills/apm-usage/commands.md | 7 +++++++ 3 files changed, 30 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fc6df1ff7f..faaef0c0c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **BREAKING:** after consumers re-vendor the shared gh-aw `apm.md`, its import requires an explicit `target` instead of deprecated `all`; `apm-action` otherwise writes `all` into the isolated `apm.yml`, where it degrades to auto-detection without harness markers. Set the workflow engine's target and recompile; see the [gh-aw migration recipe](https://microsoft.github.io/apm/integrations/gh-aw/#shared-apmmd-import-recommended). (#2706) - Re-vendored shared gh-aw workflows now default to APM 0.28.0 for both pack and restore, the version used for the recorded `microsoft/apm-action@v1.10.0` compatibility proof, not the latest CLI release; an explicit `apm-version` still overrides it. (#2706) +### Fixed + +- Generated Copilot user instructions retain every contributor's ownership; global uninstall rebuilds surviving sources and root authoring, while cleanup refusals retain receipts and report partial removal with recovery steps. (#2820) + ### Security - The shared gh-aw APM pack job now declares `contents: read` (previously `permissions: {}`), the minimum the explicit built-in-token path needs. No write scope is added, and the token is not forwarded to restore or agent jobs. (#2706) diff --git a/docs/src/content/docs/reference/targets-matrix.md b/docs/src/content/docs/reference/targets-matrix.md index ccc077370b..1ff1130b44 100644 --- a/docs/src/content/docs/reference/targets-matrix.md +++ b/docs/src/content/docs/reference/targets-matrix.md @@ -128,6 +128,25 @@ GitHub Copilot (CLI and IDE). at user scope). User-scope deploys land under `~/.copilot/`, not `~/.github/`; hook script commands are written as absolute paths so Copilot CLI can invoke them from any working directory. + + This generated file is a whole-file aggregate: the deployment ledger records + every contributing package and root owner with the final combined-content + digest. It has no editable generated regions. Author root + `*.instructions.md` files in `~/.apm/instructions/`, not in the aggregate. + + `apm uninstall -g PKG` reconstructs it from validated surviving package + sources eligible for Copilot and actual root authoring, never from generated + output. Successful cleanup and lockfile persistence remove the aggregate and + its ownership records when no eligible content remains. By default, APM + leaves unmanaged output unchanged without claiming ownership. + + Edited or unsafe aggregate cleanup is refused, retaining previous ownership + receipts. Removal is partially completed, not rolled back. Back up and move + the refused `~/.copilot/copilot-instructions.md` out of its managed path, + repair unsafe paths, then run `apm install --global`. Port wanted edits to + genuine authoring in `~/.apm/instructions/`; do not restore the old aggregate. + For source-validation or rebuild failures, fix the reported source problem + before running `apm install --global`. - **Global compile.** `apm compile -g` can also render global instructions to `~/.copilot/AGENTS.md` for root-context readers that honor `AGENTS.md`. diff --git a/packages/apm-guide/.apm/skills/apm-usage/commands.md b/packages/apm-guide/.apm/skills/apm-usage/commands.md index 1ec5ae5235..772816a051 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/commands.md +++ b/packages/apm-guide/.apm/skills/apm-usage/commands.md @@ -43,6 +43,13 @@ outside-constraint upgrades, see the `apm update` remains constraint-respecting and writable; it does not take an outside-constraint `Latest`. +`apm uninstall -g PKG` rebuilds the generated Copilot user-instructions +aggregate from validated surviving packages and root authoring. Cleanup +refusal retains previous ownership receipts after partial removal; recovery +uses `apm install --global`, not a force bypass. Follow the +[Copilot aggregate recovery steps](https://microsoft.github.io/apm/reference/targets-matrix/#copilot) +before retrying. + For `apm install -g --mcp`, mixed target selections warn and skip workspace-only runtimes. If no selected target supports user scope, the command exits `2` before creating or changing the user manifest, lockfile, or runtime From 63b3619905298efca014a6621b725b5448ffefff Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Wed, 9 Sep 2026 13:49:27 +0200 Subject: [PATCH 04/10] fix(install): share canonical cached source restoration Extract the exact cached restoration call from #2820 (824b17058826aef375c0446085197a81e1418dd8). Both independent successors inherit this same prerequisite. Move the two cached/fresh identity cases from 0119fc6fbc25fb1865f360b3edde920507b188f1 into their shared module; retain parsed URL assertions. No acquisition trust or local admission changes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/apm_cli/install/sources.py | 3 + .../install/test_cached_source_identity.py | 64 +++++++++++++++++++ 2 files changed, 67 insertions(+) create mode 100644 tests/unit/install/test_cached_source_identity.py diff --git a/src/apm_cli/install/sources.py b/src/apm_cli/install/sources.py index e5743a5e23..dbd4a456d8 100644 --- a/src/apm_cli/install/sources.py +++ b/src/apm_cli/install/sources.py @@ -470,6 +470,9 @@ def acquire(self) -> Materialization | None: package_path=install_path, source=dep_ref.repo_url, ) + from apm_cli.models.apm_package import restore_installed_package_source + + restore_installed_package_source(cached_package, dep_ref) if not cached_package.source: cached_package.source = dep_ref.repo_url diff --git a/tests/unit/install/test_cached_source_identity.py b/tests/unit/install/test_cached_source_identity.py new file mode 100644 index 0000000000..0182f3dd25 --- /dev/null +++ b/tests/unit/install/test_cached_source_identity.py @@ -0,0 +1,64 @@ +"""Cached materialization restores canonical Git source identity.""" + +from __future__ import annotations + +from pathlib import Path +from unittest.mock import MagicMock +from urllib.parse import urlparse + +import pytest + +from apm_cli.core.scope import InstallScope +from apm_cli.install.context import InstallContext +from apm_cli.install.sources import CachedDependencySource +from apm_cli.integration.targets import KNOWN_TARGETS +from apm_cli.models.dependency import DependencyReference +from apm_cli.utils.diagnostics import DiagnosticCollector +from apm_cli.utils.yaml_io import dump_yaml +from tests.utils.local_package import LocalPackageFactory + +pytestmark = pytest.mark.component + + +@pytest.mark.parametrize("fetched_this_run", [False, True], ids=["cached", "fresh"]) +def test_cached_acquisition_restores_original_git_source( + tmp_path: Path, fetched_this_run: bool +) -> None: + """Authored metadata cannot replace the Git origin used for reconstruction.""" + package = LocalPackageFactory(tmp_path).create("parent") + dump_yaml( + {"name": "parent", "version": "1.0.0", "source": "_local/forged"}, + package.manifest_path, + ) + ref = DependencyReference.parse_from_dict( + {"git": "https://gitlab.example.invalid/team/parent", "ref": "v1"} + ) + graph = MagicMock() + graph.dependency_tree.get_node.return_value = None + ctx = InstallContext( + project_root=tmp_path, + apm_dir=tmp_path, + scope=InstallScope.PROJECT, + dependency_graph=graph, + diagnostics=DiagnosticCollector(), + targets=[KNOWN_TARGETS["cursor"]], + ) + source = CachedDependencySource( + ctx, + ref, + package.root, + ref.get_unique_key(), + resolved_ref=None, + dep_locked_chk=None, + fetched_this_run=fetched_this_run, + ) + + materialized = source.acquire() + + assert materialized is not None + assert materialized.package_info is not None + restored = urlparse(materialized.package_info.package.source) + assert restored.scheme == "https" + assert restored.hostname == "gitlab.example.invalid" + assert restored == urlparse(ref.to_github_url()) + assert ctx.installed_packages[0].dep_ref is ref From f10db33963cde9c4d60359254dddaef9131e8517 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Wed, 9 Sep 2026 13:49:42 +0200 Subject: [PATCH 05/10] test(install): retain aggregate cached-source routing guard Restore the original #2820 cached-source route and mutation witness to aggregate ownership. The canonical caller and its cached/fresh behavioral cases now belong to shared prerequisite 63b3619905298efca014a6621b725b5448ffefff rather than depending on another successor landing first. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../checks/install_base_integrator_and_contraction.py | 1 + tests/integration/test_architecture_copilot_aggregate.py | 5 +++++ 2 files changed, 6 insertions(+) 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 05160e92aa..061b9b0b56 100644 --- a/scripts/architecture_linter/checks/install_base_integrator_and_contraction.py +++ b/scripts/architecture_linter/checks/install_base_integrator_and_contraction.py @@ -81,6 +81,7 @@ def check_copilot_aggregate(provider: FactsProvider) -> tuple[Violation, ...]: "restore_installed_package_source", {"to_github_url"}, ), + ("src/apm_cli/install/sources.py", "acquire", {"restore_installed_package_source"}), ( "src/apm_cli/commands/uninstall/cli.py", "uninstall", diff --git a/tests/integration/test_architecture_copilot_aggregate.py b/tests/integration/test_architecture_copilot_aggregate.py index 24ba18851a..54c4f683c7 100644 --- a/tests/integration/test_architecture_copilot_aggregate.py +++ b/tests/integration/test_architecture_copilot_aggregate.py @@ -42,6 +42,11 @@ def test_copilot_aggregate_routes_are_registered_and_satisfied() -> None: "package.source = dep_ref.to_github_url()", "package.source = None", ), + ( + "src/apm_cli/install/sources.py", + "restore_installed_package_source(cached_package, dep_ref)", + "parallel_source(cached_package, dep_ref)", + ), ( "src/apm_cli/commands/uninstall/engine.py", "rebuild_result = finalize_install_result(", From 2be4e4a2136d198d43438a5eee8a220bfe9b00ee Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Wed, 9 Sep 2026 15:14:43 +0200 Subject: [PATCH 06/10] test(install): bind aggregate repairs to existing lockfile requirements Exercise req-lk-020 fresh active-path preservation across prior identities and req-lk-016 serialized digest envelopes through canonical aggregate cleanup and attachment. Preserve the original twelve native cases and add direct uninstall from legacy last-writer-only receipts. Fold the canonical uninstall reference correction from the actual final outer4 advisory. No normative text, waiver, or runtime semantics change. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../content/docs/reference/cli/uninstall.md | 9 +- .../test_copilot_aggregate_uninstall.py | 33 +++++ .../test_aggregate_lockfile_reqs.py | 117 ++++++++++++++++++ 3 files changed, 158 insertions(+), 1 deletion(-) create mode 100644 tests/spec_conformance/test_aggregate_lockfile_reqs.py diff --git a/docs/src/content/docs/reference/cli/uninstall.md b/docs/src/content/docs/reference/cli/uninstall.md index 5ad6cd8422..c10fe9a7ee 100644 --- a/docs/src/content/docs/reference/cli/uninstall.md +++ b/docs/src/content/docs/reference/cli/uninstall.md @@ -119,7 +119,8 @@ What gets removed, in order: shared entry. Cleanup attempts every owning runtime before exiting nonzero on failure. Fix the reported configs, then run `apm install` to reconcile stale entries. -7. Lockfile entries. If no dependencies remain, `apm.lock.yaml` is deleted. +7. `apm.lock.yaml` is deleted only when no dependencies or root-owned deployment + receipts remain. Selection is atomic. If any requested identifier does not match a declaration, the command exits nonzero before lifecycle scripts or filesystem writes run. No @@ -146,6 +147,12 @@ or the on-disk `apm.lock.yaml`. Direct and orphan directories run first and may already be gone; declarations and deployed ownership remain. Resolve the listed files and retry the same uninstall command. +If APM refuses to clean up a Copilot aggregate or cannot rebuild it, packages and +declarations have already been removed. Uninstall exits nonzero and retains +ownership receipts; it does not undo the removal. Follow +[Copilot recovery](../../targets-matrix/#copilot) to resolve the output or source +errors, then run `apm install --global`. + If a managed hook changes after the initial check or is beneath a symlinked parent, uninstall preserves and lists the path. Package removal finishes, but the command exits nonzero because hook cleanup is incomplete. Inspect or repair the diff --git a/tests/integration/test_copilot_aggregate_uninstall.py b/tests/integration/test_copilot_aggregate_uninstall.py index 27f7f6b062..cb92922841 100644 --- a/tests/integration/test_copilot_aggregate_uninstall.py +++ b/tests/integration/test_copilot_aggregate_uninstall.py @@ -5,11 +5,14 @@ import hashlib import os import re +from dataclasses import replace from pathlib import Path from urllib.parse import urlparse import pytest +from apm_cli.core.deployment_ledger import DeploymentLedgerCodec +from apm_cli.core.deployment_state import DeploymentLedger from apm_cli.deps.lockfile import LockFile from apm_cli.models.dependency.selection import parse_dependency_entry from apm_cli.utils.yaml_io import dump_yaml, load_yaml @@ -67,6 +70,7 @@ def _assert_recovered_sections( "survivor-missing", "survivor-ineligible", "root-missing", + "legacy-last-writer", ], ) def test_generated_copilot_aggregate_lifecycle( @@ -179,6 +183,29 @@ def command(*args: str, expected_returncode: int = 0) -> CommandResult: if has_root: assert initial.count(_ROOT_BODY.strip()) == 1 aggregate_rel = ".copilot/copilot-instructions.md" + if obligation == "legacy-last-writer": + # Reproduce an old receipt without changing the two rendered sections. + survivor_key = refs["survivor"].get_unique_key() + legacy_record = replace(records[0], owners=(survivor_key,), active_owner=survivor_key) + DeploymentLedgerCodec.apply_to_lockfile( + DeploymentLedger(records={legacy_record.locator.key: legacy_record}), lock + ) + lock_path = isolated.config_root / "apm.lock.yaml" + lock.save(lock_path) + legacy = LockFile.read(lock_path) + assert legacy is not None + assert list(legacy.deployment_ledger.records.values()) == [legacy_record] + assert legacy.dependencies[refs["primary"].get_unique_key()].deployed_files == [] + assert legacy.dependencies[refs["primary"].get_unique_key()].deployed_file_hashes == {} + assert legacy.dependencies[survivor_key].deployed_files == [aggregate_rel] + assert legacy.dependencies[survivor_key].deployed_file_hashes == { + aggregate_rel: legacy_record.content_hash + } + assert aggregate.read_text(encoding="utf-8") == initial + assert ( + legacy_record.content_hash + == "sha256:" + hashlib.sha256(aggregate.read_bytes()).hexdigest() + ) survivor_path = refs["survivor"].get_install_path(isolated.config_root / "apm_modules") installed_instruction = survivor_path / ".apm/instructions/survivor.instructions.md" @@ -310,6 +337,12 @@ def assert_survivor_state() -> LockFile: return remaining = aggregate.read_text(encoding="utf-8") print("POST-UNINSTALL AGGREGATE", remaining) + if obligation == "legacy-last-writer": + assert bodies["primary"].strip() not in remaining, "Unrecorded removed-owner body survived" + identities = re.findall(r"", remaining) + assert [urlparse(identity) for identity in identities] == [ + urlparse(refs["survivor"].to_github_url()) + ], "Unrecorded removed-owner provenance survived" if obligation == "removed-section": assert bodies["primary"].strip() not in remaining, "Removed-owner body survived" assert "primary" not in remaining, "Removed-owner provenance survived" diff --git a/tests/spec_conformance/test_aggregate_lockfile_reqs.py b/tests/spec_conformance/test_aggregate_lockfile_reqs.py new file mode 100644 index 0000000000..d3d7806f49 --- /dev/null +++ b/tests/spec_conformance/test_aggregate_lockfile_reqs.py @@ -0,0 +1,117 @@ +"""Existing lockfile clauses exercised through aggregate install reconciliation.""" + +from __future__ import annotations + +from pathlib import Path +from types import SimpleNamespace + +import pytest + +from apm_cli.core.scope import InstallScope +from apm_cli.deps.lockfile import LockedDependency, LockFile +from apm_cli.install.phases import cleanup +from apm_cli.install.phases.lockfile import LockfileBuilder +from apm_cli.integration.targets import resolve_targets +from apm_cli.models.apm_package import APMPackage +from apm_cli.utils.diagnostics import DiagnosticCollector +from apm_cli.utils.yaml_io import load_yaml +from tests.spec_conformance._helpers import assert_spec_contains, sha256_hex + +pytestmark = pytest.mark.component + +_AGGREGATE = ".copilot/copilot-instructions.md" +_CURRENT_BYTES = ( + b"\n" + b"\nFirst contribution.\n\n" + b"\nSecond contribution.\n\n" + b"\nRoot contribution.\n\n" +) + + +@pytest.fixture +def aggregate_install_state(tmp_path: Path) -> tuple[SimpleNamespace, LockFile]: + """Model completed materialization before non-frozen cleanup and attachment.""" + target = tmp_path / _AGGREGATE + target.parent.mkdir() + target.write_bytes(_CURRENT_BYTES) + (tmp_path / "apm.yml").write_text( + "name: aggregate-conformance\nversion: 1.0.0\ntarget: copilot\n", + encoding="ascii", + ) + prior = LockFile() + prior.add_dependency( + LockedDependency( + repo_url="fixture/previous-identity", + deployed_files=[_AGGREGATE], + deployed_file_hashes={_AGGREGATE: "sha256:" + sha256_hex(_CURRENT_BYTES)}, + ) + ) + prior.local_deployed_files = [_AGGREGATE] + prior.local_deployed_file_hashes = {_AGGREGATE: "sha256:" + "a" * 64} + current = LockFile() + for name in ("first", "second"): + current.add_dependency(LockedDependency(repo_url=f"fixture/{name}")) + context = SimpleNamespace( + project_root=tmp_path, + scope=InstallScope.USER, + frozen=False, + lockfile_only=False, + only_packages=False, + existing_lockfile=prior, + intended_dep_keys=set(current.dependencies), + package_deployed_files={key: [_AGGREGATE] for key in current.dependencies}, + local_deployed_files=[_AGGREGATE], + apm_package=APMPackage.from_apm_yml(tmp_path / "apm.yml"), + targets=resolve_targets(tmp_path, user_scope=True, explicit_target=["copilot"]), + diagnostics=DiagnosticCollector(), + logger=None, + ) + return context, current + + +@pytest.mark.req("req-lk-020") +def test_nonfrozen_orphan_cleanup_preserves_fresh_aggregate( + aggregate_install_state: tuple[SimpleNamespace, LockFile], +) -> None: + """A prior different identity cannot delete the active install's fresh path.""" + context, current = aggregate_install_state + cleanup.run(context) + LockfileBuilder(context)._attach_deployed_files(current) + + assert (context.project_root / _AGGREGATE).read_bytes() == _CURRENT_BYTES + assert context.orphan_cleanup_retained == {} + assert all(dep.deployed_files == [_AGGREGATE] for dep in current.dependencies.values()) + assert current.local_deployed_files == [_AGGREGATE] + assert_spec_contains( + "During orphan cleanup, the consumer MUST preserve any path freshly\n" + "deployed by an active dependency in the current install, even when the\n" + "same path is also recorded by a prior lockfile entry under a different\n" + "dependency identity." + ) + + +@pytest.mark.req("req-lk-016") +def test_aggregate_install_serializes_current_hash_envelopes( + aggregate_install_state: tuple[SimpleNamespace, LockFile], +) -> None: + """Attachment and local-state preservation serialize the final file's digest.""" + context, current = aggregate_install_state + builder = LockfileBuilder(context) + builder._attach_deployed_files(current) + builder._preserve_existing_local_state(current) + lock_path = context.project_root / "apm.lock.yaml" + current.save(lock_path) + document = load_yaml(lock_path) + + expected = "sha256:" + sha256_hex(_CURRENT_BYTES) + assert len(document["deployments"]) == 1 + assert document["deployments"][0]["content_hash"] == expected + assert document["local_deployed_file_hashes"] == {_AGGREGATE: expected} + assert len(document["dependencies"]) == 2 + for dependency in document["dependencies"]: + assert dependency["deployed_file_hashes"] == {_AGGREGATE: expected} + assert_spec_contains( + "hash values as `:` envelopes", + "`local_deployed_file_hashes` (each value), `content_hash`,", + "writers MUST emit the explicit envelope\nform.", + ) From 475550d06088ccf05d19ae5bb53bc404387c9b4f Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Wed, 9 Sep 2026 15:31:46 +0200 Subject: [PATCH 07/10] perf(lockfile): freeze warm legacy owner records once Keep accumulation within DeploymentLedgerCodec and preserve owner order and hash fallback. Add warm root/no-root scaling traps and legacy/native semantic controls. Runtime qualification remains pending the assigned execution slot. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/apm_cli/core/deployment_ledger.py | 34 ++++--- tests/unit/core/test_deployment_state.py | 90 +++++++++++++++++++ .../test_lockfile_cross_package_reconcile.py | 85 ++++++++++++++++++ 3 files changed, 196 insertions(+), 13 deletions(-) diff --git a/src/apm_cli/core/deployment_ledger.py b/src/apm_cli/core/deployment_ledger.py index b74292ebd1..ae2bae7884 100644 --- a/src/apm_cli/core/deployment_ledger.py +++ b/src/apm_cli/core/deployment_ledger.py @@ -217,22 +217,31 @@ def from_lockfile(lockfile: LockFile) -> DeploymentLedger: ): return current - records: dict[str, DeploymentRecord] = {} + legacy_records: dict[str, tuple[DeploymentLocator, dict[str, None], str | None]] = {} for owner, dependency in lockfile.dependencies.items(): if owner == ".": continue DeploymentLedgerCodec._add_legacy_paths( - records, + legacy_records, owner, dependency.deployed_files, dependency.deployed_file_hashes, ) DeploymentLedgerCodec._add_legacy_paths( - records, + legacy_records, ".", lockfile.local_deployed_files, lockfile.local_deployed_file_hashes, ) + records = { + key: DeploymentRecord( + locator=locator, + owners=tuple(owners), + active_owner=next(reversed(owners)), + content_hash=content_hash, + ) + for key, (locator, owners, content_hash) in legacy_records.items() + } service_targets = ( ("mcp", getattr(lockfile, "mcp_target_servers", {})), ("lsp", getattr(lockfile, "lsp_target_servers", {})), @@ -726,7 +735,7 @@ def validate_rows(rows: Any) -> None: @staticmethod def _add_legacy_paths( - records: dict[str, DeploymentRecord], + records: dict[str, tuple[DeploymentLocator, dict[str, None], str | None]], owner: str, paths: list[str], hashes: dict[str, str], @@ -734,15 +743,14 @@ def _add_legacy_paths( for value in paths: locator = DeploymentLedgerCodec._legacy_locator(value) prior = records.get(locator.key) - owners = list(prior.owners) if prior else [] - if owner in owners: - owners.remove(owner) - owners.append(owner) - records[locator.key] = DeploymentRecord( - locator=locator, - owners=tuple(owners), - active_owner=owner, - content_hash=hashes.get(value) or (prior.content_hash if prior else None), + owners = prior[1] if prior else {} + # Move repeats to the end without copying an expanding owner tuple. + owners.pop(owner, None) + owners[owner] = None + records[locator.key] = ( + locator, + owners, + hashes.get(value) or (prior[2] if prior else None), ) @staticmethod diff --git a/tests/unit/core/test_deployment_state.py b/tests/unit/core/test_deployment_state.py index d31b505ef7..ab6b125c83 100644 --- a/tests/unit/core/test_deployment_state.py +++ b/tests/unit/core/test_deployment_state.py @@ -2,6 +2,8 @@ from dataclasses import replace from pathlib import Path +from types import SimpleNamespace +from urllib.parse import urlparse import pytest @@ -555,6 +557,94 @@ def test_legacy_import_and_dual_write_are_semantically_equivalent() -> None: assert rebuilt.is_semantically_equivalent(lockfile) +@pytest.mark.parametrize("value", [".copilot/copilot-instructions.md", "unknown://entry"]) +@pytest.mark.parametrize( + "first_hash,second_hash,root_hash,expected_hash", + [ + ("sha256:first", "sha256:second", None, "sha256:first"), + (None, "sha256:second", None, "sha256:second"), + (None, None, None, None), + ("sha256:first", "", "", "sha256:first"), + ("sha256:first", "sha256:second", "sha256:root", "sha256:root"), + ], +) +def test_legacy_accumulation_preserves_revisited_owner_and_hash_precedence( + monkeypatch: pytest.MonkeyPatch, + value: str, + first_hash: str | None, + second_hash: str | None, + root_hash: str | None, + expected_hash: str | None, +) -> None: + """A repeated owner moves last; absent/empty hashes retain the latest proof.""" + lock = LockFile() + for owner, content_hash in (("first", first_hash), ("second", second_hash)): + lock.add_dependency( + LockedDependency( + repo_url=f"fixture/{owner}", + deployed_files=[value, value], + deployed_file_hashes={value: content_hash} if content_hash is not None else {}, + ) + ) + entries = list(lock.dependencies.items()) + lock.local_deployed_files = [value] + lock.local_deployed_file_hashes = {value: root_hash} if root_hash is not None else {} + # Revisit an owner in the codec's input stream, including duplicate paths. + with monkeypatch.context() as patch: + patch.setattr( + lock, "dependencies", SimpleNamespace(items=lambda: [*entries, entries[0]]) + ) + ledger = DeploymentLedgerCodec.from_lockfile(lock) + assert len(ledger.records) == 1 + record = next(iter(ledger.records.values())) + assert record.owners == ("fixture/second", "fixture/first", ".") + assert record.active_owner == "." + assert record.content_hash == expected_hash + if value.startswith("unknown:"): + assert record.locator.kind == LocatorKind.URI + assert record.locator.target == "unknown" + assert urlparse(record.locator.value) == urlparse(value) + else: + assert record.locator.kind == LocatorKind.PROJECT_RELATIVE + assert record.locator.scope == "user" + assert record.locator.value == value + DeploymentLedgerCodec.apply_to_lockfile(ledger, lock) + assert DeploymentLedgerCodec.legacy_deployed_file_claims(lock) == {value: "fixture/first"} + assert all(dep.deployed_files == [value] for dep in lock.dependencies.values()) + expected_hashes = {value: expected_hash} if expected_hash is not None else {} + assert all( + dep.deployed_file_hashes == expected_hashes for dep in lock.dependencies.values() + ) + assert lock.local_deployed_file_hashes == expected_hashes + assert LockFile.from_yaml(lock.to_yaml()).deployment_ledger == ledger + + +@pytest.mark.parametrize("kind", [LocatorKind.TARGET_RELATIVE, LocatorKind.URI]) +@pytest.mark.parametrize("content_hash", [None, "sha256:current"]) +def test_canonical_native_rows_bypass_legacy_accumulation( + kind: LocatorKind, content_hash: str | None +) -> None: + """Canonical native/unknown-URI rows keep their non-last active owner and null hash.""" + locator = DeploymentLocator( + kind=kind, + target="native-fixture", + value="unknown://entry" if kind == LocatorKind.URI else "native/output.md", + runtime="fixture-runtime", + scope="user", + ) + record = DeploymentRecord( + locator=locator, + owners=("fixture/first", "fixture/second"), + active_owner="fixture/first", + content_hash=content_hash, + ) + ledger = DeploymentLedger(records={locator.key: record}) + lock = LockFile() + DeploymentLedgerCodec.apply_to_lockfile(ledger, lock, write_legacy_views=False) + assert DeploymentLedgerCodec.from_lockfile(lock) is ledger + assert DeploymentLedgerCodec.from_rows(DeploymentLedgerCodec.rows(ledger)) == ledger + + def test_legacy_shared_agents_path_has_unattributable_target() -> None: """A shared .agents root must not be attributed to the deprecated alias.""" locator = DeploymentLedgerCodec._legacy_locator(".agents/skills/demo/SKILL.md") diff --git a/tests/unit/install/phases/test_lockfile_cross_package_reconcile.py b/tests/unit/install/phases/test_lockfile_cross_package_reconcile.py index 0207966466..904c8bcc7a 100644 --- a/tests/unit/install/phases/test_lockfile_cross_package_reconcile.py +++ b/tests/unit/install/phases/test_lockfile_cross_package_reconcile.py @@ -14,7 +14,9 @@ from __future__ import annotations +from pathlib import Path from types import SimpleNamespace +from typing import Any import pytest @@ -137,6 +139,89 @@ def reconcile(**kwargs): ) +@pytest.mark.parametrize("has_root", [False, True], ids=["no-root", "root"]) +def test_warm_aggregate_root_carry_work_is_linear( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, has_root: bool +) -> None: + """Count actual codec owner freezing through the existing-local-state chain.""" + from apm_cli.core import deployment_ledger as codec + from apm_cli.core.deployment_state import DeploymentLedger, DeploymentRecord + from apm_cli.core.scope import InstallScope + from apm_cli.install.phases.lockfile import compute_deployed_hashes + from apm_cli.integration.targets import resolve_targets + + observations = [] + relative = ".copilot/copilot-instructions.md" + original_record = codec.DeploymentRecord + for size in (50, 500): + root = tmp_path / str(size) + aggregate = root / relative + aggregate.parent.mkdir(parents=True) + aggregate.write_bytes(b"current contribution\n" * size) + current_hash = compute_deployed_hashes([relative], root)[relative] + current, previous = LockFile(), LockFile() + claims = {} + for index in range(size): + owner = f"fixture/package-{index}" + current.add_dependency(LockedDependency(repo_url=owner)) + previous.add_dependency(LockedDependency(repo_url=owner)) + claims[owner] = [relative] + owners = (*claims, ".") if has_root else tuple(claims) + locator = codec.DeploymentLedgerCodec._legacy_locator(relative) + prior_record = DeploymentRecord( + locator=locator, + owners=owners, + active_owner=owners[-1], + content_hash="sha256:" + "a" * 64, + ) + codec.DeploymentLedgerCodec.apply_to_lockfile( + DeploymentLedger(records={locator.key: prior_record}), previous + ) + ctx = _ctx( + package_deployed_files=claims, + existing_lockfile=previous, + targets=resolve_targets(root, user_scope=True, explicit_target=["copilot"]), + project_root=root, + ) + ctx.scope = InstallScope.USER + ctx.local_deployed_files = [relative] if has_root else [] + ctx.logger = None + builder = LockfileBuilder(ctx) + builder._attach_deployed_files(current) + counts = {"records": 0, "owners": 0} + + def record(**kwargs: Any) -> DeploymentRecord: + counts["records"] += 1 + counts["owners"] += len(kwargs["owners"]) + return original_record(**kwargs) + + with monkeypatch.context() as patch: + patch.setattr(codec, "DeploymentRecord", record) + builder._preserve_existing_local_state(current) + observations.append(dict(counts)) + assert counts["records"] > 0, "The warm codec route was not exercised" + assert len(current.deployment_ledger.records) == 1 + final = next(iter(current.deployment_ledger.records.values())) + assert final.owners == owners + assert final.active_owner == owners[-1] + assert final.content_hash == current_hash + assert all(dep.deployed_files == [relative] for dep in current.dependencies.values()) + assert all( + dep.deployed_file_hashes == {relative: current_hash} + for dep in current.dependencies.values() + ) + assert current.local_deployed_files == ([relative] if has_root else []) + assert current.local_deployed_file_hashes == ( + {relative: current_hash} if has_root else {} + ) + restored = LockFile.from_yaml(current.to_yaml()) + assert restored.deployment_ledger == current.deployment_ledger + small, large = observations + print("WARM CODEC OWNER WORK", observations) + assert large["owners"] < 15 * small["owners"] + assert large["records"] == small["records"] + + def _target(name, root_dir=".claude"): return SimpleNamespace(name=name, root_dir=root_dir, primitives={}) From 8b538b92cfa378105fafb392b04cd7b0ff28475b Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Wed, 9 Sep 2026 19:45:09 +0200 Subject: [PATCH 08/10] test(spec): record aggregate lockfile conformance witnesses Regenerate the existing conformance statement after adding the req-lk-016 and req-lk-020 aggregate regression tests. Preserve all requirement statuses and normative text while making the committed test inventory match hosted generation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- CONFORMANCE.json | 6 ++++-- CONFORMANCE.md | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/CONFORMANCE.json b/CONFORMANCE.json index b2fbc74e4d..a992842ed2 100644 --- a/CONFORMANCE.json +++ b/CONFORMANCE.json @@ -232,8 +232,9 @@ "keyword": "MUST", "section": "5.2", "status": "active", - "test_count": 1, + "test_count": 2, "tests": [ + "tests/spec_conformance/test_aggregate_lockfile_reqs.py::test_aggregate_install_serializes_current_hash_envelopes", "tests/spec_conformance/test_lockfile_reqs.py::test_lockfile_reader_tolerates_bare_hex_hash" ] }, @@ -279,8 +280,9 @@ "keyword": "MUST", "section": "5.2", "status": "active", - "test_count": 3, + "test_count": 4, "tests": [ + "tests/spec_conformance/test_aggregate_lockfile_reqs.py::test_nonfrozen_orphan_cleanup_preserves_fresh_aggregate", "tests/spec_conformance/test_lockfile_reqs.py::TestFinalLockfileTargetContraction::test_removes_dropped_target_instruction_file", "tests/spec_conformance/test_lockfile_reqs.py::test_cli_target_without_manifest_targets_preserves_inactive_state", "tests/spec_conformance/test_lockfile_reqs.py::test_lockfile_reconciles_inactive_target_paths_fail_safe" diff --git a/CONFORMANCE.md b/CONFORMANCE.md index a6c0ce79fd..78d3050313 100644 --- a/CONFORMANCE.md +++ b/CONFORMANCE.md @@ -56,11 +56,11 @@ Repository-coordinate segments are case-insensitive for `github.com`, GitHub Ent | [req-lk-013](docs/src/content/docs/specs/openapm-v0.1.md#req-lk-013) | MUST | 5.2 | consumer | active | 2 | - | | [req-lk-014](docs/src/content/docs/specs/openapm-v0.1.md#req-lk-014) | MUST | 5.2 | consumer | active | 1 | - | | [req-lk-015](docs/src/content/docs/specs/openapm-v0.1.md#req-lk-015) | MUST | 5.6.4 | consumer | active | 1 | - | -| [req-lk-016](docs/src/content/docs/specs/openapm-v0.1.md#req-lk-016) | MUST | 5.2 | consumer | active | 1 | - | +| [req-lk-016](docs/src/content/docs/specs/openapm-v0.1.md#req-lk-016) | MUST | 5.2 | consumer | active | 2 | - | | [req-lk-017](docs/src/content/docs/specs/openapm-v0.1.md#req-lk-017) | MUST | 5.2 | consumer | active | 1 | - | | [req-lk-018](docs/src/content/docs/specs/openapm-v0.1.md#req-lk-018) | SHOULD | 5.5 | consumer | active | 1 | - | | [req-lk-019](docs/src/content/docs/specs/openapm-v0.1.md#req-lk-019) | MUST | 5.2 | consumer | active | 1 | - | -| [req-lk-020](docs/src/content/docs/specs/openapm-v0.1.md#req-lk-020) | MUST | 5.2 | consumer | active | 3 | - | +| [req-lk-020](docs/src/content/docs/specs/openapm-v0.1.md#req-lk-020) | MUST | 5.2 | consumer | active | 4 | - | | [req-lk-021](docs/src/content/docs/specs/openapm-v0.1.md#req-lk-021) | MUST | 5.2 | consumer | active | 2 | - | | [req-lk-022](docs/src/content/docs/specs/openapm-v0.1.md#req-lk-022) | MUST | 5.2 | consumer | active | 4 | - | | [req-mf-001](docs/src/content/docs/specs/openapm-v0.1.md#req-mf-001) | MUST | 4.1 | producer | active | 1 | - | From 0f5a6a4f884cd127728aaf38dff06fa5ca0b8de1 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Wed, 9 Sep 2026 19:51:32 +0200 Subject: [PATCH 09/10] test(install): keep aggregate regression fixtures lint clean Extract persisted legacy receipt, installed-source, and root-only assertions without dropping checks; bind warm codec counters outside the size loop. Preserve all thirteen native scenarios and scaling-control semantics. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../test_copilot_aggregate_uninstall.py | 109 +++++++++++------- .../test_lockfile_cross_package_reconcile.py | 19 ++- 2 files changed, 79 insertions(+), 49 deletions(-) diff --git a/tests/integration/test_copilot_aggregate_uninstall.py b/tests/integration/test_copilot_aggregate_uninstall.py index cb92922841..c048e187a0 100644 --- a/tests/integration/test_copilot_aggregate_uninstall.py +++ b/tests/integration/test_copilot_aggregate_uninstall.py @@ -14,6 +14,7 @@ from apm_cli.core.deployment_ledger import DeploymentLedgerCodec from apm_cli.core.deployment_state import DeploymentLedger from apm_cli.deps.lockfile import LockFile +from apm_cli.models.dependency.reference import DependencyReference from apm_cli.models.dependency.selection import parse_dependency_entry from apm_cli.utils.yaml_io import dump_yaml, load_yaml from tests.utils.apm_lifecycle_runner import ApmLifecycleRunner, CommandResult @@ -33,6 +34,56 @@ _ROOT_BODY = "# Root contribution\nKeep the actual root-local instruction.\n" +def _assert_installed_sources( + lock: LockFile, + refs: dict[str, DependencyReference], + commits: dict[str, str], + sources: dict[str, dict[str, bytes]], + modules_root: Path, +) -> None: + """Check the installed declarations, commits, and authored bytes.""" + assert {dep.get_unique_key() for dep in lock.get_package_dependencies()} == { + ref.get_unique_key() for ref in refs.values() + } + for name, ref in refs.items(): + locked = lock.dependencies[ref.get_unique_key()] + assert locked.resolved_commit == commits[name] + assert locked.to_dependency_ref().get_unique_key() == ref.get_unique_key() + for relative, content in sources[name].items(): + assert (ref.get_install_path(modules_root) / relative).read_bytes() == content + + +def _persist_legacy_last_writer_receipt( + lock: LockFile, + lock_path: Path, + aggregate: Path, + primary_key: str, + survivor_key: str, +) -> None: + """Persist a survivor-only old receipt without changing rendered sections.""" + initial = aggregate.read_text(encoding="utf-8") + record = next(iter(lock.deployment_ledger.records.values())) + legacy_record = replace(record, owners=(survivor_key,), active_owner=survivor_key) + DeploymentLedgerCodec.apply_to_lockfile( + DeploymentLedger(records={legacy_record.locator.key: legacy_record}), lock + ) + lock.save(lock_path) + legacy = LockFile.read(lock_path) + assert legacy is not None + assert list(legacy.deployment_ledger.records.values()) == [legacy_record] + assert legacy.dependencies[primary_key].deployed_files == [] + assert legacy.dependencies[primary_key].deployed_file_hashes == {} + aggregate_rel = ".copilot/copilot-instructions.md" + assert legacy.dependencies[survivor_key].deployed_files == [aggregate_rel] + assert legacy.dependencies[survivor_key].deployed_file_hashes == { + aggregate_rel: legacy_record.content_hash + } + assert aggregate.read_text(encoding="utf-8") == initial + assert ( + legacy_record.content_hash == "sha256:" + hashlib.sha256(aggregate.read_bytes()).hexdigest() + ) + + def _assert_recovered_sections( aggregate: Path, survivor_source: str, survivor_body: str, removed_body: str ) -> None: @@ -55,6 +106,17 @@ def _assert_recovered_sections( assert recovered.count(_ROOT_BODY.strip()) == 1 +def _assert_root_only_state(aggregate: Path, lock_path: Path) -> None: + """Verify exact root content and its sole surviving receipt.""" + assert aggregate.read_text(encoding="utf-8") == ( + f"{_HEADER}\n\n{_ROOT_BODY.strip()}\n\n" + ) + root_lock = LockFile.read(lock_path) + assert root_lock is not None + assert root_lock.get_package_dependencies() == [] + assert [record.owners for record in root_lock.deployment_ledger.records.values()] == [(".",)] + + @pytest.mark.parametrize( "obligation", [ @@ -151,15 +213,7 @@ def command(*args: str, expected_returncode: int = 0) -> CommandResult: lock = LockFile.read(isolated.config_root / "apm.lock.yaml") assert lock is not None expected_keys = {refs[name].get_unique_key() for name in refs} - assert {dep.get_unique_key() for dep in lock.get_package_dependencies()} == expected_keys - for name, ref in refs.items(): - locked = lock.dependencies[ref.get_unique_key()] - assert locked.resolved_commit == commits[name] - assert locked.to_dependency_ref().get_unique_key() == ref.get_unique_key() - for relative, content in sources[name].items(): - assert ( - ref.get_install_path(isolated.config_root / "apm_modules") / relative - ).read_bytes() == content + _assert_installed_sources(lock, refs, commits, sources, isolated.config_root / "apm_modules") assert load_yaml(isolated.config_root / "apm.yml") == manifest initial = aggregate.read_text(encoding="utf-8") print("INITIAL AGGREGATE", initial) @@ -184,27 +238,12 @@ def command(*args: str, expected_returncode: int = 0) -> CommandResult: assert initial.count(_ROOT_BODY.strip()) == 1 aggregate_rel = ".copilot/copilot-instructions.md" if obligation == "legacy-last-writer": - # Reproduce an old receipt without changing the two rendered sections. - survivor_key = refs["survivor"].get_unique_key() - legacy_record = replace(records[0], owners=(survivor_key,), active_owner=survivor_key) - DeploymentLedgerCodec.apply_to_lockfile( - DeploymentLedger(records={legacy_record.locator.key: legacy_record}), lock - ) - lock_path = isolated.config_root / "apm.lock.yaml" - lock.save(lock_path) - legacy = LockFile.read(lock_path) - assert legacy is not None - assert list(legacy.deployment_ledger.records.values()) == [legacy_record] - assert legacy.dependencies[refs["primary"].get_unique_key()].deployed_files == [] - assert legacy.dependencies[refs["primary"].get_unique_key()].deployed_file_hashes == {} - assert legacy.dependencies[survivor_key].deployed_files == [aggregate_rel] - assert legacy.dependencies[survivor_key].deployed_file_hashes == { - aggregate_rel: legacy_record.content_hash - } - assert aggregate.read_text(encoding="utf-8") == initial - assert ( - legacy_record.content_hash - == "sha256:" + hashlib.sha256(aggregate.read_bytes()).hexdigest() + _persist_legacy_last_writer_receipt( + lock, + isolated.config_root / "apm.lock.yaml", + aggregate, + refs["primary"].get_unique_key(), + refs["survivor"].get_unique_key(), ) survivor_path = refs["survivor"].get_install_path(isolated.config_root / "apm_modules") installed_instruction = survivor_path / ".apm/instructions/survivor.instructions.md" @@ -356,15 +395,7 @@ def assert_survivor_state() -> LockFile: assert remaining.count(_ROOT_BODY.strip()) == 1, "Actual root-local contribution lost" assert root_instruction.read_text(encoding="utf-8") == _ROOT_BODY command("uninstall", declarations[1]["git"], "--global") - assert aggregate.read_text(encoding="utf-8") == ( - f"{_HEADER}\n\n{_ROOT_BODY.strip()}\n\n" - ) - root_lock = LockFile.read(isolated.config_root / "apm.lock.yaml") - assert root_lock is not None - assert root_lock.get_package_dependencies() == [] - assert [record.owners for record in root_lock.deployment_ledger.records.values()] == [ - (".",) - ] + _assert_root_only_state(aggregate, isolated.config_root / "apm.lock.yaml") else: survivor = refs["survivor"] lock = assert_survivor_state() diff --git a/tests/unit/install/phases/test_lockfile_cross_package_reconcile.py b/tests/unit/install/phases/test_lockfile_cross_package_reconcile.py index 904c8bcc7a..7e03c33f22 100644 --- a/tests/unit/install/phases/test_lockfile_cross_package_reconcile.py +++ b/tests/unit/install/phases/test_lockfile_cross_package_reconcile.py @@ -153,6 +153,13 @@ def test_warm_aggregate_root_carry_work_is_linear( observations = [] relative = ".copilot/copilot-instructions.md" original_record = codec.DeploymentRecord + counts = {"records": 0, "owners": 0} + + def record(**kwargs: Any) -> DeploymentRecord: + counts["records"] += 1 + counts["owners"] += len(kwargs["owners"]) + return original_record(**kwargs) + for size in (50, 500): root = tmp_path / str(size) aggregate = root / relative @@ -188,13 +195,7 @@ def test_warm_aggregate_root_carry_work_is_linear( ctx.logger = None builder = LockfileBuilder(ctx) builder._attach_deployed_files(current) - counts = {"records": 0, "owners": 0} - - def record(**kwargs: Any) -> DeploymentRecord: - counts["records"] += 1 - counts["owners"] += len(kwargs["owners"]) - return original_record(**kwargs) - + counts.update(records=0, owners=0) with monkeypatch.context() as patch: patch.setattr(codec, "DeploymentRecord", record) builder._preserve_existing_local_state(current) @@ -211,9 +212,7 @@ def record(**kwargs: Any) -> DeploymentRecord: for dep in current.dependencies.values() ) assert current.local_deployed_files == ([relative] if has_root else []) - assert current.local_deployed_file_hashes == ( - {relative: current_hash} if has_root else {} - ) + assert current.local_deployed_file_hashes == ({relative: current_hash} if has_root else {}) restored = LockFile.from_yaml(current.to_yaml()) assert restored.deployment_ledger == current.deployment_ledger small, large = observations From 6ccd935af3bd8bd02cbd889db742f41f3c030896 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Wed, 9 Sep 2026 19:59:59 +0200 Subject: [PATCH 10/10] style(tests): format aggregate codec semantic controls Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- tests/unit/core/test_deployment_state.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/unit/core/test_deployment_state.py b/tests/unit/core/test_deployment_state.py index ab6b125c83..92e14eae8b 100644 --- a/tests/unit/core/test_deployment_state.py +++ b/tests/unit/core/test_deployment_state.py @@ -591,9 +591,7 @@ def test_legacy_accumulation_preserves_revisited_owner_and_hash_precedence( lock.local_deployed_file_hashes = {value: root_hash} if root_hash is not None else {} # Revisit an owner in the codec's input stream, including duplicate paths. with monkeypatch.context() as patch: - patch.setattr( - lock, "dependencies", SimpleNamespace(items=lambda: [*entries, entries[0]]) - ) + patch.setattr(lock, "dependencies", SimpleNamespace(items=lambda: [*entries, entries[0]])) ledger = DeploymentLedgerCodec.from_lockfile(lock) assert len(ledger.records) == 1 record = next(iter(ledger.records.values())) @@ -612,9 +610,7 @@ def test_legacy_accumulation_preserves_revisited_owner_and_hash_precedence( assert DeploymentLedgerCodec.legacy_deployed_file_claims(lock) == {value: "fixture/first"} assert all(dep.deployed_files == [value] for dep in lock.dependencies.values()) expected_hashes = {value: expected_hash} if expected_hash is not None else {} - assert all( - dep.deployed_file_hashes == expected_hashes for dep in lock.dependencies.values() - ) + assert all(dep.deployed_file_hashes == expected_hashes for dep in lock.dependencies.values()) assert lock.local_deployed_file_hashes == expected_hashes assert LockFile.from_yaml(lock.to_yaml()).deployment_ledger == ledger