Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .apm/architecture/owners/install-deployment.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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"]
},
{
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,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)
Expand Down
6 changes: 4 additions & 2 deletions CONFORMANCE.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
]
},
Expand Down Expand Up @@ -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"
Expand Down
4 changes: 2 additions & 2 deletions CONFORMANCE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 | - |
Expand Down
9 changes: 8 additions & 1 deletion docs/src/content/docs/reference/cli/uninstall.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
19 changes: 19 additions & 0 deletions docs/src/content/docs/reference/targets-matrix.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.

Expand Down
7 changes: 7 additions & 0 deletions packages/apm-guide/.apm/skills/apm-usage/commands.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,141 @@

_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/install/sources.py", "acquire", {"restore_installed_package_source"}),
(
"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`."""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down Expand Up @@ -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).",
Expand Down
35 changes: 32 additions & 3 deletions src/apm_cli/commands/uninstall/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from ...models.apm_package import APMPackage
from ...utils.path_security import PathTraversalError
from .engine import (
AggregateIntegrationError,
IntegrationCleanupOutcome,
LocalSurvivorRefreshError,
MCPUninstallCleanupError,
Expand Down Expand Up @@ -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}"
)
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
Loading