feat(apmx): run packaged contracts through native Copilot - #2935
Open
Daniel Meppiel (danielmeppiel) wants to merge 8 commits into
Open
feat(apmx): run packaged contracts through native Copilot#2935Daniel Meppiel (danielmeppiel) wants to merge 8 commits into
Daniel Meppiel (danielmeppiel) wants to merge 8 commits into
Conversation
Recover the bounded contract engine from closed #2837 and add the bundled apmx entrypoint, private package preparation, locked source replay, independent assessment, companion distribution, and documented lifecycle coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Daniel Meppiel (danielmeppiel)
requested a review
from Sergio Sisternes (sergio-sisternes-epam)
as a code owner
September 10, 2026 07:08
Copilot started reviewing on behalf of
Daniel Meppiel (danielmeppiel)
September 10, 2026 07:08
View session
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Critical security findings and multiple moderate correctness and distribution issues remain unresolved.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Lite
Findings: 2
New issues introduced by this change (5)
| Severity | Finding |
|---|---|
 |
src/apm_cli/contracts/process.py — This bypasses the repository's trusted Git boundary: shutil.which("git") can select a… |
|
src/apm_cli/contracts/records.py — package_ref is copied verbatim into the durable run record. Git/HTTPS references can contain… |
 |
install.ps1 — This probe only protects the apmx launcher, but pip install --user apm-cli also installs the… |
|
install.sh — The pip fallback installs the package that provides both apm and apmx, but this ownership probe… |
|
src/apm_cli/contracts/engine.py — Exceptions from logger.close() or the preceding store.update() escape before… |
What changed in this PR
Adds the bundled apmx entrypoint for packaged native Copilot contract execution, including preparation, planning, evidence capture, distribution, and documentation.
Changes:
- Adds package selection, validation, lock handling, and contract execution.
- Records artifacts, assessments, source identities, and run state.
- Bundles
apmxwith frozen builds and ownership-safe installers. - Adds examples, documentation, signing, and regression coverage.
Unresolved findings include critical Git trust-boundary and credential-retention issues, moderate signing, installer, execution, finalization, and lock-validation issues, plus two nits.
| File | Reviewed change |
|---|---|
tests/unit/test_windows_signing_contract.py |
Windows signing contract coverage |
tests/unit/test_windows_installer_launchers.py |
Windows launcher coverage |
tests/unit/test_unix_installer_checksums.py |
Unix archive integrity coverage |
tests/unit/test_pip_companion_ownership.py |
Pip launcher ownership coverage |
tests/unit/test_enterprise_bootstrap_installers.py |
Bootstrap installer coverage |
tests/unit/test_build_spec.py |
Frozen build layout coverage |
tests/unit/install/test_unix_install_ownership.py |
Unix installer ownership coverage |
tests/unit/contracts/test_terminal_pty.py |
Terminal PTY behavior |
tests/unit/contracts/test_pty_lifecycle.py |
PTY lifecycle behavior |
tests/unit/contracts/test_example_checks.py |
Example checker behavior |
tests/unit/contracts/test_dispatch.py |
Contract dispatch behavior |
tests/unit/contracts/test_contract_policy.py |
Contract policy checks |
tests/unit/contracts/test_cli_lifecycle.py |
CLI lifecycle behavior |
tests/unit/contracts/test_architecture.py |
Architecture enforcement |
tests/unit/contracts/test_apmx_optimized.py |
Optimized apmx launcher behavior |
tests/unit/contracts/test_apmx_acquisition_validation.py |
Acquisition validation |
src/apm_cli/utils/yaml_io.py |
Bounded YAML and frontmatter handling |
src/apm_cli/utils/console.py |
Contract console output helpers |
src/apm_cli/utils/atomic_io.py |
Durable atomic writes; wording nit remains |
src/apm_cli/runtime/registry.py |
Runtime registration |
src/apm_cli/runtime/copilot_runtime.py |
Native Copilot adapter |
src/apm_cli/runtime/base.py |
Runtime interface |
src/apm_cli/policy/discovery.py |
Policy discovery |
src/apm_cli/policy/contract_prerequisite.py |
Contract prerequisites |
src/apm_cli/models/validation.py |
Package validation models |
src/apm_cli/install/contract_source_validation.py |
Contract source validation |
src/apm_cli/deps/github_downloader.py |
GitHub package acquisition |
src/apm_cli/deps/artifactory_orchestrator.py |
Artifactory source orchestration |
src/apm_cli/contracts/records.py |
Durable records; critical secret-retention finding remains |
src/apm_cli/contracts/process.py |
Process supervision; critical executable-boundary finding remains |
src/apm_cli/contracts/models.py |
Contract data models |
src/apm_cli/contracts/events.py |
Contract lifecycle events |
src/apm_cli/contracts/engine.py |
Contract lifecycle; moderate environment, finalization, and lock-validation findings remain |
src/apm_cli/contracts/__init__.py |
Contract package initialization |
src/apm_cli/commands/run.py |
Legacy run behavior |
src/apm_cli/commands/plan.py |
Contract planning |
src/apm_cli/commands/contracts.py |
Contract command dispatch |
src/apm_cli/cli.py |
CLI integration |
src/apm_cli/apmx.py |
Bundled apmx entrypoint |
scripts/windows/sign-binary.ps1 |
Windows binary signing |
scripts/windows/build-binary.ps1 |
Windows binary packaging |
scripts/build-binary.sh |
Unix binary packaging |
scripts/architecture_linter/groups/contracts_tests.py |
Contract test architecture group |
scripts/architecture_linter/checks/contract_leaf_runtime.py |
Contract runtime boundary checks |
pyproject.toml |
Adds the apmx script |
packages/apm-guide/.apm/skills/apm-usage/package-authoring.md |
Contract authoring guidance |
packages/apm-guide/.apm/skills/apm-usage/commands.md |
Command usage guidance |
install.sh |
Unix installation and ownership checks; moderate finding remains |
examples/contracts/reuse-contract/handoff.contract.md |
Reusable contract example |
examples/contracts/reuse-contract/apm.yml |
Reusable example configuration |
examples/contracts/README.md |
Contract examples guide |
examples/contracts/packaged-job/skills/handoff-style/SKILL.md |
Packaged skill example |
examples/contracts/packaged-job/skills/handoff-style/apm.yml |
Packaged skill configuration |
examples/contracts/packaged-job/README.md |
Packaged execution walkthrough |
examples/contracts/packaged-job/notes.md |
Packaged fixture notes |
examples/contracts/packaged-job/contracts/handoff.contract.md |
Packaged contract |
examples/contracts/packaged-job/checks/check_handoff.py |
Packaged checker |
examples/contracts/packaged-job/caller/notes.md |
Caller input fixture |
examples/contracts/packaged-job/apm.yml |
Packaged job configuration |
examples/contracts/handoff-style/SKILL.md |
Handoff skill example |
examples/contracts/handoff-style/apm.yml |
Handoff skill configuration |
examples/contracts/first-contract/notes.md |
First contract fixture notes |
examples/contracts/first-contract/handoff.contract.md |
First contract example |
examples/contracts/first-contract/checks/check_handoff.py |
First contract checker |
examples/contracts/first-contract/apm.yml |
First contract configuration |
docs/src/content/docs/reference/cli/run.md |
Run command documentation |
docs/src/content/docs/reference/cli/plan.md |
Plan command documentation |
docs/src/content/docs/reference/cli/apmx.md |
apmx documentation; CLI index nit remains |
docs/src/content/docs/consumer/run-contracts.md |
Consumer contract walkthrough |
docs/astro.config.mjs |
Documentation navigation |
build/apm.spec |
Frozen apm and apmx packaging |
.gitignore |
Contract run-state exclusions |
.github/workflows/build-release.yml |
Release build and signing workflow; moderate finding remains |
.apm/docs-index.yml |
Documentation indexing |
.apm/architecture/owners/contracts-tooling.json |
Contract tooling ownership |
Suppressed comments (5)
.github/workflows/build-release.yml:142
- This signing invocation is unreachable when the certificate secret is configured: the step's preceding condition reads
env.WINDOWS_CERT_PFX, but that variable is only declared inside the step'senvblock and is therefore empty whenifis evaluated. Since the build step also does not receive the certificate, Windows release artifacts can never be Authenticode-signed through this workflow. Let the script run for every Windows build (it already exits successfully when no certificate is present), or move the secret to a scope available to the condition.
run: |
uv run pwsh scripts/windows/sign-binary.ps1
docs/src/content/docs/reference/cli/apmx.md:9
- The new
apmxpage is not represented in the authoritativedocs/src/content/docs/reference/index.mdCLI table: its run row still lists onlyrun. Although the autogenerated sidebar exposes the page, users following the reference landing page cannot discover the bundled entrypoint from the canonical command overview; add it there as part of this CLI documentation change.
Bundled with APM, `apmx` runs one explicit contract on one harness, captures one
output, runs independent checks, and retains a record.
src/apm_cli/contracts/engine.py:81
- Frozen
apmxruns pass PyInstaller's rewrittenLD_LIBRARY_PATH/DYLD_*variables to every checker because thisProcessRequestleavesenvasNone. A checker such as the documentedpython3command can then fail to load against bundled libraries on affected hosts; use the existingexternal_process_env()helper here, as the Copilot adapter already does.
src/apm_cli/contracts/engine.py:149 - Package preparation validates the caller's direct lock only before
invoke_contract; this re-plan reuses the already-preparedContractSourceand never re-reads that caller lock.plan_contracthashes the prepared package lock, not the caller lock that selected the package, so a caller lock/declared-reference change between preparation and execution can be missed while the run proceeds on stale source identity. Carry the caller pin identity into the plan and compare it here, or reacquire/revalidate the package source before launching.
src/apm_cli/utils/atomic_io.py:94 - The final guarantee is inaccurate for
durable=True: a directory-sync failure is raised afteros.replace, so the destination has already changed. Qualify this statement to failures before replacement; otherwise callers are given an incorrect atomicity guarantee for the new durable path.
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Repair legacy downloader and Windows fixture setup, register the shipped CLI and architecture surfaces, and bind caller-lock replay to req-rs-015 without weakening the conformance gate. Address Copilot and panel findings by preserving both pip launchers, redacting retained references, using trusted system Git, and repairing transcript finalization failures. Regression mutations prove the changed guards. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
Author
APM Review Panel:
|
| Persona | B | R | N | Takeaway |
|---|---|---|---|---|
| python-architect | 0 | 1 | 1 | Sound single-owner design with bounded leaf-runtime guardrails; a small wrapper-duplication nit is not a shipping fault. |
| cli-logging-expert | 0 | 1 | 0 | Finalization OSError can be mislabeled as package preparation failure. |
| devx-ux-expert | 0 | 2 | 1 | Documented bounded CLI; harness-choice hint and companion discoverability could improve. |
| supply-chain-security-expert | 0 | 1 | 0 | Pip ownership guard protects apmx but omits the sibling apm launcher. |
| oss-growth-hacker | 0 | 0 | 0 | Scoped example docs are honest about limits; no conversion or release work needed. |
| auth-expert | 0 | 0 | 0 | Reviewer found no auth regression and considered local_git remote-free. |
| doc-writer | 0 | 0 | 0 | No additional high-confidence documentation ship regressions beyond the known index omission. |
| test-coverage-expert | 2 | 0 | 0 | Raised post-prepare lock drift and credential retention coverage concerns. |
| performance-expert | 0 | 0 | 1 | No high-confidence performance regression in one bounded packaged job. |
B/R/N are original finding strengths, not shipping gates. Concrete items below are already folded; speculative/cosmetic items stay out of this recovery.
Recommendation
All 21 latest-head checks are successful, neutral or intentionally skipped at 126d83488c9b4a0a79959ef4b753416d7ae037e6. Two bounded recovery pushes converged. No additional product or architecture work is recommended. No merge or auto-merge was performed or authorized.
Per-persona findings and disposition
python-architect
- Downloaded-package validation wrappers are duplicated. Both call the canonical validate_apm_package owner; suggested extraction is minor DRY cleanup.
- The new contracts leaf-runtime static guard and tests are well targeted. Positive finding: concrete bypass shapes are guarded.
cli-logging-expert
- Handle logger.close and record-update failures through the finalization owner. These fail after native execution; the existing generic OSError message names the wrong phase.
devx-ux-expert
- Make --on enumerate copilot or improve the invalid-harness hint. Current generic unknown-runtime message omits the supported choice.
- Link apmx from the reference index. Standalone companion is otherwise absent from the command landing table.
- Replace raw OSError text in commands/contracts.py with stable wording. Small diagnostic polish.
supply-chain-security-expert
- Check ownership of both console scripts before pip fallback. pip installs apm and apmx into the selected user scheme; either foreign launcher must be preserved.
oss-growth-hacker
- No additional finding.
auth-expert
- No additional finding.
doc-writer
- No additional finding.
test-coverage-expert
- No caller lock recheck between prepared plan and engine execution. Reviewer proposes mutating the caller lock after internal preparation; driver must distinguish supported CLI plan/run reuse from an internal-object TOCTOU scenario.
- Persisted package_ref needs credential redaction coverage. Record construction copies raw reference; established redactor should be reused.
performance-expert
- A small installed skill manifest is parsed twice. Sub-millisecond redundant computation; optional cleanup, not shipping risk.
Folded in this run
- Frozen system Git needs trusted executable lookup and external-process library-path cleanup. --
3b41322daf. - The dependency parser accepts credentialed URLs; raw retained references could expose those values. --
3b41322daf. - pip installs both launchers; the Windows fallback must protect foreign apm as well as apmx. --
3b41322daf. - pip installs both launchers; the Unix fallback must protect foreign apm as well as apmx. --
3b41322daf. - Transcript close and record-phase update failures must repair the incomplete finalization record. --
3b41322daf. - Repair downloader/Windows fixtures, rule inventory, and CLI reference parity including apmx. --
3b41322daf. - Bind package consumer lock replay to existing req-rs-015 and regenerate conformance artifacts without a waiver. --
3b41322daf. - Correct the apm ownership target to shipped cli:main and derive test entrypoint metadata from pyproject.toml. --
126d83488c.
Copilot signals reviewed
src/apm_cli/contracts/process.py:234-- LEGIT, fixed in3b41322daf; answered in-thread and resolved.src/apm_cli/contracts/records.py:108-- LEGIT, fixed in3b41322daf; answered in-thread and resolved.install.ps1:336-- LEGIT, fixed in3b41322daf; answered in-thread and resolved.install.sh:1334-- LEGIT, fixed in3b41322daf; answered in-thread and resolved.src/apm_cli/contracts/engine.py:265-- LEGIT, fixed in3b41322daf; answered in-thread and resolved.
Deferred (out-of-scope follow-ups)
- Extract downloader validation wrappers -- Cosmetic call-site DRY cleanup, not a demonstrated regression; both already route through the validation owner.
- Cache the second small skill-manifest parse -- Microoptimization outside actual CI recovery; one bounded skill, no measured delivery regression.
- Change harness choices and generic filesystem wording -- Diagnostic polish is outside the explicit CI/high-confidence-regression filter; no new CLI behavior is needed to ship.
- Add a post-prepare caller-lock transaction guarantee -- Speculative internal-object/concurrent-edit scenario, not supported persisted-plan reuse; would add a new transaction contract.
No backlog issues were created for these suggestions.
Regression-trap evidence (mutation-break gate)
- Removing the new behavioral guards caused 9 failures; restoring them passed.
- Disabling the registered contract-owner guard caused 14 bypass-rejection failures; restored.
- Final-head functional evidence: 64 passed, 3 skipped, covering all 13 detected owners, the real PowerShell stable-path consumer, source-lock replay, CLI reference parity, and every recovery guard. The broader first recovery suite passed 900 tests (19 skipped, 2 subtests passed).
- Final consistency review caught and corrected an incorrect
apmownership target before completion: the fixture now reads actualpyproject.tomlmetadata; using:cliinstead of shipped:maincauses its owned-launcher test to fail. - Spec suite: 204 passed, 2 skipped. Actual docs build: 127 pages / 1,056 links, no broken relative links; all 35 CLI reference pages matched live commands.
- Version-2 canonical-owner schema and semantic verifier passed for all 13 touched decisions, using executed exact-head test IDs.
Lint contract
Full ruff check/format, pylint R0801, auth/architecture boundaries, YAML I/O, portable paths and 2,100-line guards passed. Used the existing .venv without syncing dependencies or changing uv.lock; latest main was already integrated.
CI
All 21 latest-head checks observed SUCCESS/NEUTRAL/SKIPPED. Runs: https://github.com/microsoft/apm/actions/runs/34482816821; https://github.com/microsoft/apm/actions/runs/34482816847; https://github.com/microsoft/apm/actions/runs/34482816951; https://github.com/microsoft/apm/actions/runs/34482816955; https://github.com/microsoft/apm/actions/runs/34482816959; https://github.com/microsoft/apm/actions/runs/34482817047; https://github.com/microsoft/apm/actions/runs/34482817088. Full snapshot: latest-pr.json.
Mergeability status
| PR | Head | Advisory | Iterations | Folds | Deferrals | Copilot rounds | CI | Mergeable | GitHub state | Note |
|---|---|---|---|---|---|---|---|---|---|---|
| #2935 | 126d834 | ship_with_followups | 1 | 8 | 4 | 2 | green | MERGEABLE | BLOCKED | GitHub metadata reported verbatim; no merge action |
The pip ownership fixture now reads actual pyproject console-script metadata, so it catches an incorrect ownership target instead of repeating that mistake. Preserve existing owned APM launchers while still refusing foreign replacements. The owned-launcher test fails with the incorrect mapping and passes with the shipped apm_cli.cli:main entrypoint. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Require 'apm experimental enable contracts' before local or packaged contract planning and execution, while leaving legacy script runs unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> # Conflicts: # .apm/architecture/owners/contracts-tooling.json # CONFORMANCE.md # src/apm_cli/models/validation.py
Use neutral output, explicit Copilot attribution, concise previews and copyable result paths. Preserve live streaming and terminal cleanup. Keep passing checks visible while consistently recording native unsandboxed runs as UNPROVEN/21, with rejection and halt precedence unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add
apmxas a bundled APM entrypoint for one packaged job, one explicit contract, one native harness, and an artifact with independent assessment evidence. Recovers the bounded execution engine from closed #2837 and completes package preparation and companion distribution without introducing a second engine.apm experimental enable contracts apmx --from PACKAGE_REF contracts/job.contract.md \ --on copilot --model gpt-6-astra --allow-host-accessImportant
A completed native run now returns UNPROVEN / exit 21 even when all contract checks pass: this runner does not enforce host isolation. Individual check results and the output remain available. Host-access consent permits execution; it does not upgrade the assurance result. Earlier experimental builds returned VERIFIED / 0 for passing local checks.
Changes
.apm/runs/.apmxin Python and frozen distributions, including ownership-safe installers that preserve foreign executables.apm experimental enable contracts; name per-run consent--allow-host-accessto explain the permission rather than the internal execution profile.apm runscript behavior; add examples, consumer/reference documentation, and regression coverage.Latest CLI/result follow-up evidence
main. Refreshed GitHub CI status is not asserted here.tests/unit/contracts/test_pty_lifecycle.py,test_terminal_pty.pytests/unit/contracts/test_logger.pytests/unit/contracts/test_reliability.py,test_engine.py,tests/integration/test_apmx_packaged_job.pyPreviously completed evidence
These results precede the native-assurance ceiling correction above; historical VERIFIED/0 outcomes are not the current runner's semantics.
apmxfetched the original Git-hosted example atf906a748e1f021218cfe19d43429bdc12046d8c7, ran native Copilot, and finished VERIFIED / exit 0 with 1/1 original checks passing under the earlier implementation.Boundaries and trade-offs
Native execution is Copilot/POSIX and advisory, not sandboxed.
--allow-host-accesspermits Copilot and checks to use host files, network and available login details for this run; it does not override policy. The earlier--allow-advisoryspelling belonged to this unmerged experimental feature and is replaced, not retained as an alias. Result enum values and record field names are unchanged; native outcome reduction now caps otherwise-passing runs at UNPROVEN/21, without falsely recording a process stop.Check success is not factual correctness, signed provenance, or merge authorization. No sandbox implementation, full graph/factory scheduler, retries/resume, or delivery automation is added. Global Copilot configuration remains unchanged; earlier native proof used an invocation-local
COPILOT_HOME. Windows native runtime/signing was not exercised locally.This is a new PR; #2837 remains closed. No release or version bump is included.
How to try
Follow
examples/contracts/packaged-job/README.mdfrom an independent caller directory, then inspect the reported artifact andrecord.json. Expect passing individual checks and an overall UNPROVEN/21 for native execution. Use--planfor offline inspection; missing unmaterialized sources/imports refuse without fetching. To execute, omit--planand supply--allow-host-access. During a run, observe Copilot's public activity above the spinner;NO_COLORorAPM_PROGRESS=neverdisables animation. Use--verbosefor diagnostic detail.Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com