Feature/dotnet port#84
Closed
seiggy wants to merge 4 commits intomicrosoft:mainfrom
Closed
Conversation
Full port of the APM CLI from Python to .NET 10, packaged as a dnx tool for install-free execution. Includes: - Spectre.Console.Cli for CLI framework (maps to Python Click+Rich) - 89 source files across Commands, Models, Core, Compilation, Dependencies, Integration, Primitives, Runtime, Adapters, Workflow - 38 xUnit test files with 1173 tests (FakeItEasy + AwesomeAssertions) - NativeAOT support for standalone binaries (5 RIDs) - Central Package Version Management (Directory.Packages.props) - Cross-platform PowerShell 7+ install script - Source-generated JSON serialization for AOT compatibility - Full adapter wiring: VSCode, Codex, Copilot MCP server configuration - Conflict detection, safe installation, auto-install virtual packages - PackAsTool=true with ToolCommandName=apm, PackageId=apm-cli
Mirrors Python pipeline structure: - Multi-platform test matrix (ubuntu, macos, windows) - NativeAOT builds on correct OS runners (no cross-compile) - Integration tests with built binaries - Release validation in isolated environment - NuGet package publishing - Triggered on v*.*.* tags and src/apm-dotnet/** changes
Add --single-agents, --verbose/-v, --local-only, --clean flags that exist in the Python CLI but were missing from docs. Restore accurate --with-constitution/--no-constitution description.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a complete .NET 10 port of the APM CLI, providing a cross-platform implementation that matches the Python version's functionality while adding NativeAOT support for standalone binaries.
Changes:
- Full .NET 10 port with 89 source files (~16K LOC) covering Commands, Models, Core, Compilation, Dependencies, Integration, Primitives, Runtime, Adapters, Workflow, and Registry modules
- Comprehensive test suite with 38 test files containing 1173 xUnit tests (0 failures)
- Cross-platform PowerShell 7+ install script replacing bash-only installation
- Updated CLI reference documentation with missing command flags
Reviewed changes
Copilot reviewed 111 out of 152 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/apm-dotnet/tests/Apm.Cli.Tests/Commands/SelectiveInstallTests.cs | Unit tests for selective install filter matching logic |
| src/apm-dotnet/tests/Apm.Cli.Tests/Apm.Cli.Tests.csproj | Test project configuration with xUnit and FakeItEasy |
| src/apm-dotnet/tests/Apm.Cli.Tests/Adapters/*.cs | Adapter integration tests for VSCode, Codex, and client factory |
| src/apm-dotnet/src/Apm.Cli/Workflow/*.cs | Workflow execution, parsing, and discovery implementation |
| src/apm-dotnet/src/Apm.Cli/Utils/*.cs | Utility classes for versioning, GitHub host handling, JSON serialization, and console helpers |
| src/apm-dotnet/src/Apm.Cli/Runtime/*.cs | Runtime adapter implementations for Copilot, Codex, and LLM |
| src/apm-dotnet/src/Apm.Cli/Registry/*.cs | Registry client and integration for MCP package discovery |
| src/apm-dotnet/src/Apm.Cli/Program.cs | Main CLI entry point with Spectre.Console.Cli configuration |
| src/apm-dotnet/src/Apm.Cli/Primitives/PrimitiveParser.cs | Parser for primitive definition files with YAML frontmatter |
| src/apm-dotnet/src/Apm.Cli/Output/*.cs | Output formatters and models for compilation results |
| src/apm-dotnet/src/Apm.Cli/Models/*.cs | Core data models for packages, dependencies, and enums |
| src/apm-dotnet/src/Apm.Cli/Integration/*.cs | Integration modules for transforming and installing skills, prompts, agents, and commands |
| src/apm-dotnet/src/Apm.Cli/Dependencies/*.cs | Dependency resolution, verification, locking, and collection parsing |
| src/apm-dotnet/src/Apm.Cli/Core/*.cs | Core operations including target detection, safe installation, configuration, and factories |
| src/apm-dotnet/src/Apm.Cli/Compilation/*.cs | Compilation infrastructure for AGENTS.md generation with constitution injection |
| src/apm-dotnet/src/Apm.Cli/Commands/*.cs | Command implementations for init, install, compile, run, preview, list, and deps subcommands |
| src/apm-dotnet/src/Apm.Cli/Apm.Cli.csproj | Main project file with NativeAOT configuration and dnx tool packaging |
| src/apm-dotnet/src/Apm.Cli/Adapters/*.cs | Client and package manager adapter interfaces and implementations |
| src/apm-dotnet/. | Solution files, build configuration, and package management setup |
| docs/cli-reference.md | Documentation updates for missing CLI flags and auto-install feature |
GetCurrentConfig() returns JsonNode values from JSON parsing but ListInstalled/Uninstall were checking for Dictionary<string, object?>. Added GetServersSection() helper that handles both JsonObject and JsonNode types. Tests now assert correct behavior instead of documenting the bug.
Collaborator
|
@seiggy — this is genuinely impressive work. 34K lines, 1173 tests, NativeAOT, full feature parity — the engineering effort is clear and I appreciate it. However, I’m going to close this. The reason is straightforward: APM cannot sustain two codebases. The project is actively evolving — we’re adding new format detection, changing architectural patterns, and iterating on the manifest weekly. Every change would need to be implemented, tested, and reviewed twice. With the current team size, that’s not realistic, and the codebases would drift immediately. The Python implementation isn’t a limitation right now. We ship standalone binaries via PyInstaller across all platforms. Users run apm install — they don’t interact with the runtime. If there’s genuine community demand for a .NET implementation, maintaining it as an independent fork is absolutely an option — but it shouldn’t live in the main repo where it creates a maintenance obligation. Thank you for the effort. Closing with respect, not dismissal. |
danielmeppiel
added a commit
that referenced
this pull request
Apr 29, 2026
… test asserts - pipeline.py:301 — pass ctx.auth_resolver to _preflight_auth_check instead of the local arg, which can be None even after resolve phase populates ctx.auth_resolver (resolve.py:91-92). Prevents an AttributeError on update installs that don't pass an AuthResolver explicitly. Flagged by Copilot reviewer. - 3 test files — replace 'dev.azure.com' in str(...) with bounded full-phrase equality assertion against 'Authentication failed for dev.azure.com'. Satisfies CodeQL #82/#83/#84 (incomplete URL substring sanitization) and our own tests.instructions.md rule banning bare URL/host substring checks. Tests: 20/20 green in tests/unit/install/. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
danielmeppiel
added a commit
that referenced
this pull request
Apr 29, 2026
- pipeline.py:301 -- pass ctx.auth_resolver to _preflight_auth_check instead of the local 'auth_resolver' arg, which can be None even after resolve phase populates ctx.auth_resolver (resolve.py:91-92). Prevents AttributeError on update installs that don't pass an AuthResolver explicitly. Flagged by Copilot reviewer. - 3 test files -- replace 'dev.azure.com' in str(...) with bounded full-phrase equality assertion against 'Authentication failed for dev.azure.com'. Satisfies CodeQL alerts #82/#83/#84 (incomplete URL substring sanitization) and our own tests.instructions.md rule banning bare URL/host substring checks. Verified: 20/20 unit tests in tests/unit/install/test_*.py green. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
9 tasks
danielmeppiel
added a commit
that referenced
this pull request
Apr 29, 2026
…) (#1031) * fix(workflows): skip-don't-fail panel label gate; bump gh-aw v0.68.3 -> v0.71.1 Replace the on.steps: 'exit 1' label-name guards in pr-review-panel and triage-panel with top-level frontmatter 'if:' fields. gh-aw propagates top-level 'if:' to BOTH the pre_activation and activation jobs, so unmatched label events now render as a clean gray Skipped status instead of red Failed (which was polluting the CI dashboard on every PR labeled with anything other than 'panel-review', and on every issue labeled with anything other than 'status/needs-triage'). Workaround for the lack of native label-name filtering on pull_request_target / issues 'labeled' triggers. Both .md files now carry a TODO marker pointing at github/gh-aw ADR-28737, which adds a first-class 'on.labels:' filter (committed 2026-04-27, post-v0.71.1, not yet released). Once released, both gates can collapse to 'on.labels: [<name>]'. Also bump gh-aw v0.68.3 -> v0.71.1 (latest released) and recompile all workflows. Other lock.yml files and agentics-maintenance.yml change only because of the setup-action SHA bump and the regenerated maintenance-workflow template; no behavioural change there. Repro of the original noise: https://github.com/microsoft/apm/actions/runs/25089778042 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(install): add AuthenticationError to install/errors.py (#1015) Typed exception for remote-host auth failures (PAT rejected, bearer rejected, no credentials available). Carries a pre-rendered diagnostic_context from build_error_context so the renderer can display actionable guidance on the default output path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(install): thread auth_scheme + git_env through validation.py (#1015) Three defects fixed: 1. Pass auth_scheme from _dep_ctx to _build_repo_url so bearer tokens use extraheader injection instead of embedding JWT in URL userinfo. 2. Merge _dep_ctx.git_env (GIT_CONFIG_COUNT/KEY_0/VALUE_0 overrides) into the subprocess env so git ls-remote sends the Authorization header for AAD bearer tokens. 3. Detect auth-related failures (401/403/Authentication failed) from git ls-remote stderr and raise AuthenticationError with build_error_context diagnostics instead of returning bare False. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(install): re-raise AuthenticationError + --update pre-flight probe (#1015) - Add AuthenticationError to the re-raise chain (alongside PolicyViolationError, DirectDependencyError, PathTraversalError) so auth failures are not wrapped as "Failed to resolve APM dependencies: ...". - Add _preflight_auth_check(): when update_refs is set, run one git ls-remote per distinct (host, org) cluster BEFORE any write phase. Aborts with AuthenticationError carrying "No files were modified" + build_error_context diagnostics. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(install): render AuthenticationError on default output path (#1015) Add except AuthenticationError handlers at both install entry points in commands/install.py. Renders the diagnostic_context (from build_error_context) on the DEFAULT path -- not --verbose-gated. Catches AuthenticationError BEFORE the generic Exception handler so the "Failed to install APM dependencies:" double-wrap is avoided. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test(install): unit tests for ADO bearer auth + AuthenticationError (#1015) - test_errors.py: AuthenticationError attribute roundtrip, isinstance - test_validation_ado_bearer.py: bearer scheme passed to _build_repo_url, git_env merged into subprocess, 401/403 raise AuthenticationError, DNS failure returns False, PAT regression (basic scheme embeds token) - test_pipeline_auth_preflight.py: --update pre-flight rejects bad auth with "No files were modified", passes good auth, skips github.com, deduplicates (host, org) clusters - test_install_cmd_auth_rendering.py: AuthenticationError not caught by PolicyViolationError, bypasses generic RuntimeError wrap - Fix: let AuthenticationError propagate through outer try/except in _validate_package_exists (the catch-all was for parse failures only) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test(install): integration tests for #1015 ADO auth regression - bearer-only install (no PAT) succeeds via az cli - bogus PAT + no az -> actionable diagnostic, no legacy wording - --update pre-flight abort: "No files were modified", files untouched - explicit PAT regression after bearer fix Note: integration tests require live ADO access + az login; skipped in CI unless APM_TEST_ADO_BEARER=1 and tenant context is configured. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(auth): lead ADO section with az login, add tenant-discovery guidance - Reorder ADO quick-start to recommend `az login` first, PAT second - Add tenant-discovery guidance (org settings URL + az account show) - Document auth-failure diagnostics and --update pre-flight behavior - Mirror changes to skill resource (authentication.md sync) Closes #1015 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chore: CHANGELOG entry for #1015 + trim install.py to LOC budget Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(install): address PR #1031 review (#1015) - pipeline.py:301 -- pass ctx.auth_resolver to _preflight_auth_check instead of the local 'auth_resolver' arg, which can be None even after resolve phase populates ctx.auth_resolver (resolve.py:91-92). Prevents AttributeError on update installs that don't pass an AuthResolver explicitly. Flagged by Copilot reviewer. - 3 test files -- replace 'dev.azure.com' in str(...) with bounded full-phrase equality assertion against 'Authentication failed for dev.azure.com'. Satisfies CodeQL alerts #82/#83/#84 (incomplete URL substring sanitization) and our own tests.instructions.md rule banning bare URL/host substring checks. Verified: 20/20 unit tests in tests/unit/install/test_*.py green. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Daniel Meppiel <danielmeppiel@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.
🚀 New Feature
Description
Full port of the APM CLI from Python to .NET 10, packaged as a
dnxtool for install-free execution viadnx run apm-cli. The CLI command isapm(matching the Python version).Supports Linux, macOS, and Windows with NativeAOT standalone binaries.
Key highlights
win-x64,linux-x64,linux-arm64,osx-x64,osx-arm64Directory.Packages.propsinstall.sh.NET Library Mapping
Feature Parity
Validated across 27 parity checks covering all modules. 0 critical gaps. 4 moderate test coverage gaps and 9 minor remain:
Moderate (test coverage, not missing features):
CompileAgentsMdstatic convenience method defaults differ slightly_get_validation_suggestion/_display_validation_errorsCLI helpers not portedDetectRuntimesFromScripts(parse apm.yml scripts for runtime detection) not portedMinor: Progress callback, timeout tests, trailing slash normalization edge cases, process abstraction for runtime unit testing, and a few Python-specific module tests.
Changes Made
src/apm-dotnet/— solution, projects, 89 source files)docs/cli-reference.md— added missing CLI flags).github/workflows/build-release-dotnet.yml)src/apm-dotnet/install.ps1)Testing
Checklist
enhancementorfeaturelabel to this PR