fix: allow global direct MCP installs (closes #2548) - #2734
fix: allow global direct MCP installs (closes #2548)#2734Daniel Meppiel (danielmeppiel) wants to merge 8 commits into
Conversation
There was a problem hiding this comment.
Copilot review overview
Review tier: Lite
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
src/apm_cli/commands/install.py — In _handle_mcp_install, you compute resolved_registry_url using the full precedence chain (flag… |
What changed in this PR
This PR fixes the apm install -g --mcp NAME direct-install path so it honors user (global) scope instead of being rejected and/or writing to project-scoped state, aligning the direct MCP branch with the standard install pipeline’s single scope decision.
Changes:
- Compute
InstallScopeonce ininstall()(from--global) and thread it through the direct--mcphandler. - Remove the blanket
--globalrejection fromvalidate_mcp_conflicts. - Add regression + architecture guardrails (unit/integration tests + boundary lint), and update docs/usage references to describe user-scope behavior and workspace-only skips.
| File | Description |
|---|---|
| src/apm_cli/commands/install.py | Computes scope once and routes direct --mcp installs through scope-aware manifest/apm-dir paths. |
| src/apm_cli/install/mcp/conflicts.py | Removes the obsolete “--global not supported” conflict from the MCP conflict matrix. |
| tests/unit/test_install_command.py | Adds a unit regression test asserting -g --mcp mutates only the user manifest and threads scope to the integrator. |
| tests/unit/install/test_mcp_conflicts.py | Removes E2 coverage tied to the old global-scope rejection. |
| tests/integration/test_wave2_adapters_coverage.py | Updates integration coverage to match the revised MCP conflict matrix API/behavior. |
| tests/integration/test_config_surface_lifecycle_contract.py | Adds an e2e scenario verifying global direct MCP installs update ~/.apm/apm.yml and user runtime config (Claude). |
| tests/integration/test_architecture_authorities.py | Adds an AST-based guard ensuring the direct MCP path consumes the install command’s scope decision. |
| scripts/lint-architecture-boundaries.sh | Adds a static boundary lint enforcing single-owner scope routing for direct MCP installs. |
| docs/src/content/docs/reference/cli/install.md | Updates --global docs to describe -g --mcp user-manifest routing and workspace-only skip behavior. |
| docs/src/content/docs/consumer/install-mcp-servers.md | Clarifies that direct global installs read/update ~/.apm/apm.yml (no project fallback). |
| packages/apm-guide/.apm/skills/apm-usage/commands.md | Updates CLI usage guidance to reflect user-scope MCP behavior for -g --mcp. |
| .github/instructions/architecture.instructions.md | Registers “Install command scope selection” as a canonical owner entry. |
| .apm/instructions/architecture.instructions.md | Mirrors the same architecture canonical-owner addition for in-repo APM instructions. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| apm_dir=mcp_apm_dir, | ||
| scope=mcp_scope, | ||
| scope=scope, | ||
| registry_url=validated_registry_url, |
APM Review Panel:
|
| Persona | B | R | N | Takeaway |
|---|---|---|---|---|
| Python Architect | 1 | 0 | 0 | Fix user-scope target resolution; the default global MCP path still scans the project. |
| CLI Logging Expert | 0 | 1 | 0 | Missing user-manifest recovery guidance points to a project-only command. |
| DevX UX Expert | 1 | 1 | 0 | Targetless discovery and unsupported-only selections violate the documented flow. |
| Supply Chain Security Expert | 2 | 1 | 0 | Preserve HTTPS defaults and prevent registry credential exposure. |
| OSS Growth Hacker | 0 | 1 | 0 | Give the restored workflow a changelog discovery path. |
| Auth Expert | 2 | 0 | 0 | Ambient registry URLs can bypass HTTP safeguards and expose credentials. |
| Doc Writer | 0 | 1 | 1 | Docs are accurate; add Hermes parity to two runtime lists. |
| Test Coverage Expert | 1 | 1 | 0 | Add targetless and persisted-registry e2e coverage. |
B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.
Top 5 follow-ups
- [Test Coverage Expert] (blocking-severity) Thread user scope through target resolution and add a targetless global MCP e2e regression test -- the current path inherits project signals.
- [Supply Chain Security Expert] (blocking-severity) Preserve registry source provenance, require explicit HTTP opt-in, and prevent ambient URL credentials from persistence or logs -- this consolidates the Security and Auth findings.
- [OSS Growth Hacker] Add a concise Unreleased changelog entry for restored global direct MCP installs -- this is mandatory release hygiene for user-visible behavior.
- [DevX UX Expert] Preflight unsupported global target sets before mutating the user manifest -- a failed install should not leave partial global state.
- [CLI Logging Expert] Provide accurate recovery guidance when the user manifest is missing --
apm initcreates project state, not~/.apm/apm.yml.
Architecture
classDiagram
direction LR
class InstallCommand {
<<ApplicationAdapter>>
+install(global_, mcp_name, target)
+_handle_mcp_install(scope, target)
}
class InstallScope {
<<Enum>>
PROJECT
USER
}
class ScopePaths {
<<CanonicalOwner>>
+get_manifest_path(scope) Path
+get_apm_dir(scope) Path
}
class TargetResolver {
<<CanonicalOwner>>
+resolve_manifest_target_decision(root, manifest_path, explicit_target) EffectiveTargetDecision
}
class MCPCommand {
<<Orchestrator>>
+run_mcp_install(scope, target_decision)
}
class MCPIntegrator {
<<Facade>>
+install(scope, user_scope, target_decision)
}
class ScopeBoundaryGuard {
<<StaticBoundaryCheck>>
+check_install_scope_selection(provider)
}
InstallCommand ..> InstallScope : computes once
InstallCommand ..> ScopePaths : resolves paths
InstallCommand ..> TargetResolver : resolves targets
InstallCommand ..> MCPCommand : dispatches
MCPCommand ..> MCPIntegrator : delegates
ScopeBoundaryGuard ..> InstallCommand : verifies
note for TargetResolver "Gap: user-scope intent is not forwarded"
class InstallCommand:::touched
class ScopeBoundaryGuard:::touched
classDef touched fill:#fff3b0,stroke:#d47600
flowchart TD
A["apm install -g --mcp NAME"] --> B["install(): scope = InstallScope.USER"]
B --> C["validate_mcp_conflicts()"]
C --> D["_handle_mcp_install(scope=USER)"]
D --> E["[I/O] get_manifest_path(USER): ~/.apm/apm.yml"]
E --> F{"resolve_manifest_target_decision(): target source"}
F -->|explicit or manifest| G["EffectiveTargetDecision"]
F -->|none| H["[I/O] current gap: resolve_targets(Path.cwd())"]
H --> I["Project markers or exit 2"]
G --> J["[FS] run_mcp_install(): user manifest and runtime config"]
I --> J
J --> K["[LOCK] update ~/.apm/apm.lock.yaml"]
Recommendation
Address targetless scope resolution and ambient registry security, add durable regression coverage, and include the changelog line before reassessing. Keep policy-root behavior as a lower-priority investigation unless focused evidence proves it incorrect.
Full per-persona findings
Python Architect
- [blocking] Global MCP target resolution still uses project-scope auto-detection at
src/apm_cli/commands/install.py:766.
Forwarduser_scopethrough the canonical resolver and extend the e2e and static guard.
CLI Logging Expert
- [recommended] Missing user-manifest recovery guidance recommends
apm init, which creates project state.
DevX UX Expert
- [blocking] Targetless global MCP installs inherit project detection.
- [recommended] Unsupported-only global target sets can mutate the user manifest before reporting failure.
Supply Chain Security Expert
- [blocking] Ambient registry configuration can implicitly opt into plaintext HTTP.
- [blocking] Environment URL credentials can reach manifests and verbose logs.
- [recommended] Policy discovery root parity needs a focused test before changing semantics.
OSS Growth Hacker
- [recommended] Add the restored command to the Unreleased changelog.
Auth Expert
- [blocking] Ambient HTTP registry URLs bypass the separate opt-in.
- [blocking] Ambient registry credentials can be persisted and printed.
Doc Writer
- [recommended] Add Hermes-when-enabled parity to two global runtime lists.
- [nit] The necessary documentation additions slightly increase word count.
Test Coverage Expert
- [blocking] Add targetless global MCP e2e coverage with a conflicting project signal.
- [recommended] Persisted registry routing currently has only mocked unit coverage.
Performance Expert -- inactive
Straight-line routing, linter, tests, and docs do not touch a performance surface.
This panel is advisory. It does not block merge. Re-apply the
panel-review label after addressing feedback to re-run.
c4b047c to
9eb44f5
Compare
APM Spec Guardian:
|
| Panel | Stance | Shocked | New B | New R | New N |
|---|---|---|---|---|---|
| Swagger Editor | ship_with_followups | 8/10 | 0 | 2 | 1 |
| OCI Editor | ship_with_followups | 8/10 | 0 | 1 | 0 |
| Package Manager Editor | needs_next_brief | 6/10 | 1 | 1 | 0 |
| TAG Architect | ship | 9/10 | 0 | 0 | 1 |
B = new blocking-severity findings, R = new recommended, N = new nits.
Counts are signal strength, not gates. The maintainer ships.
Convergent themes
- T1 -- Environment-independent target bootstrap tests (supporting: sw-rec-r2-2, oci-rec-r2-1)
- T2 -- Exact
allsemantics and correct target anchor (supporting: sw-nit-r2-1, pkg-rec-r2-1, tag-nit-r2-1)
Fold now (5 items)
- [F1 / standalone] req-tg-014 -- Retain the explicit MUST that missing declared capability means unsupported.
Success criterion:req-tg-014contains the explicit MUST. - [F2 / T2] Section 4.2.1 -- Retain explicit
allexpansion and the#421-targetlink.
Success criterion: no#421-targetsreference remains. - [F3 / standalone] req-tg-014 mixed persistence -- Retain supported, replay-equivalent target identifiers and forbid unsupported persistence or remapping.
Success criterion: a second targetless replay selects the same supported set. - [F4 / standalone] target bootstrap -- Retain bootstrap construction from
supported_runtimes.
Success criterion: bootstrap does not persist original unsupported aliases. - [F5 / T1] bootstrap tests -- Retain deterministic discovery and mixed replay assertions.
Success criterion: targetless and mixed-set focused tests pass.
Linter notes (1 scope note)
- [11] Non-spec Python files are part of this PR; the general apm-review-panel ran in parallel. Checks 1-10 pass, including ASCII, forbidden tokens, schemas, fixtures, 120 unique anchors, count reconciliation, links, fixture citations, and changelog coverage. Mermaid check skipped because the spec contains zero Mermaid blocks.
Linter handoff: req-tg-014 uses MUST;
#421-targetresolves; mixed persistence is replay-equivalent; bootstrap usessupported_runtimes; the conformance suite reports 178 passed and 2 skipped; normative count remains 120 (115 MUST, 5 SHOULD).
Full round-2 panel findings
Swagger Editor -- shocked 8/10, confidence high
- [recommended] Make the missing-capability rule explicitly normative. Folded.
- [recommended] Remove the environment-dependent targetless unit assertion. Folded.
- [nit] Correct the Section 4.2.1 fragment. Folded.
OCI Editor -- shocked 8/10, confidence high
- Round-1 pre-mutation issue closed.
- [recommended] Make the targetless unit deterministic. Folded.
Package Manager Editor -- shocked 6/10, confidence high
- [blocking] Persisting the original mixed selection could change runtime selection on replay. Folded after panel into spec, implementation, and test.
- [recommended] Define
allbehavior directly and fix its link. Folded.
TAG Architect -- shocked 9/10, confidence high
- Both round-1 scope and capability-contract issues closed.
- [nit] Correct the Section 4.2.1 fragment. Folded.
This panel is advisory. It does not block merge. Re-apply the spec-review label after addressing feedback to re-run.
APM Review Panel:
|
| Persona | B | R | N | Takeaway |
|---|---|---|---|---|
| Python Architect | 2 | 0 | 0 | Two cross-cutting owner consolidations should move to dedicated architecture work. |
| CLI Logging Expert | 0 | 0 | 0 | Output is actionable, redacted, scope-aware, and dry-run aware. |
| DevX UX Expert | 0 | 1 | 0 | Persisted HTTP recovery guidance was folded. |
| Supply Chain Security Expert | 2 | 0 | 0 | Strict HTTP parsing and ambient provenance findings were folded. |
| OSS Growth Hacker | 0 | 0 | 0 | Changelog, docs, and runnable evidence form a complete adoption story. |
| Auth Expert | 1 | 0 | 0 | Embedded client userinfo rejection was folded. |
| Doc Writer | 0 | 2 | 0 | HTTP opt-in and precise no-mutation wording were folded. |
| Test Coverage Expert | 0 | 1 | 0 | The excluded-target recovery assertion was folded. |
B = blocking-severity findings, R = recommended, N = nits.
Counts show signals raised during the terminal pass; the sections below record their disposition.
Top 2 follow-ups
- [Python Architect] Introduce one immutable MCP target plan shared by command and integrator -- this removes duplicate planning but requires a cross-module API migration.
- [Python Architect] Centralize registry parsing and safe display in a
RegistryEndpointvalue object -- this requires registry-wide migration across config, install, client, and command surfaces.
Recommendation
Ship the focused fix with green CI, then track the canonical MCP target plan and RegistryEndpoint consolidation as dedicated architecture work.
Folded in this run
- (Copilot) Apply persisted registry configuration during direct MCP integration -- resolved in
8ab1773351. - (panel) Port the scope owner guard to the sharded architecture linter -- resolved in
1f8da5be26. - (panel) Thread user scope through target resolution and discover from the user deploy root -- resolved in
2dd2a5d752. - (panel) Reject zero-supported, excluded, and disabled experimental targets before writes -- resolved in
5e6ae8e628. - (panel) Filter mixed target sets and persist replay-equivalent supported targets -- resolved in
5bc5cdc745. - (panel) Keep global direct MCP dry-runs write-free -- resolved in
2dd2a5d752. - (panel) Require exact ambient/config HTTP opt-in and pin validated registry provenance -- resolved in
6d04d3600b. - (panel) Reject and redact userinfo, query, fragment, and malformed-port values end to end -- resolved in
6d04d3600b. - (panel) Add real-binary target, registry, redaction, replay, exclusion, and dry-run coverage -- resolved in
6d04d3600b. - (panel) Add req-tg-014, generated conformance disclosure, changelog, CLI docs, and apm-guide parity -- resolved in
64be60be4b.
Copilot signals reviewed
src/apm_cli/commands/install.py:844-- LEGIT: persistedapm configregistry selection was diagnosed but not applied during integration (resolved in8ab1773351).
Deferred (out-of-scope follow-ups)
- (panel) Introduce one immutable MCP target plan -- scope boundary: PR scope is one global direct-MCP bug; this requires a cross-module planner API migration across every MCP install caller.
- (panel) Centralize registry parsing and safe display -- scope boundary: PR scope is direct global MCP behavior; this requires registry-wide migration across config, install, client, and command surfaces.
Regression-trap evidence (mutation-break gate)
- Targetless user discovery and the static scope owner failed when
user_scope=is_user_scope(scope)was removed; restored. - Mixed, excluded, disabled-experimental, and zero-supported target tests failed when their pre-write guards were removed; restored.
- Configured and ambient registry tests failed when provenance or exact HTTP opt-in was removed; restored.
- Dry-run state tests failed when the preview return was bypassed; restored.
- Userinfo, query, fragment, and malformed-port tests failed when rejection/redaction guards were removed; restored.
- Generated req-tg-014 disclosure test failed when the disclosure was removed; restored.
Lint contract
The full local CI mirror passed at the exact final head: ruff check and format silent, YAML/file-length/relative-path guards clean, pylint R0801 10.00/10, auth-signal and architecture boundary lints clean.
CI
All 18 checks are conclusive on 6d04d3600bb1b3ef120b9e70d40e109b2c88e6d7: 17 SUCCESS and 1 SKIPPED. CI run: https://github.com/microsoft/apm/actions/runs/33481602427. Spec conformance: https://github.com/microsoft/apm/actions/runs/33481602377. One CI recovery iteration repaired the initial Mode B spec-citation failure.
Mergeability status
| PR | head SHA | CEO stance | iters | folds | defers | Copilot rounds | CI | mergeable | mergeStateStatus | notes |
|---|---|---|---|---|---|---|---|---|---|---|
| #2734 | 6d04d36 |
ship_with_followups | 4 | 12 | 2 | 2 | green | MERGEABLE | BLOCKED | pending required review |
Convergence
4 outer iterations; 2 Copilot rounds. Final panel stance: ship_with_followups. All concrete in-scope follow-ups were folded; two scope-crossing refactors are recorded above.
Ready for maintainer review.
Full terminal disposition
- Security/auth findings: folded and verified by unit plus real-binary tests.
- DevX/docs findings: folded into CLI help, Starlight reference/consumer pages, config/MCP reference, and the shipped command resource.
- Test evidence: 1341 affected tests and 13 subtests passed; 179 spec tests passed with 2 skipped; latest GitHub CI is green.
- Architecture consolidation findings: deferred with explicit scope boundaries.
This panel is advisory. It does not block merge. Re-apply the panel-review label after addressing feedback to re-run.
Shepherd-driver terminal advisory: converged at reviewed headPR #2734 completed the Phase 4 convergence loop at Folded in this run
Copilot signal reviewed
Deferred (out-of-scope follow-ups)
Canonical-owner and functional evidence
Lint and CIThe full exact-head lint mirror passed: ruff check and format silent; YAML, file-length, and relative-path guards clean; pylint R0801 10.00/10; auth-signal and architecture boundary lints clean. All 18 checks are conclusive on the reviewed head: 17 SUCCESS and 1 SKIPPED. CI run: https://github.com/microsoft/apm/actions/runs/33481602427. Mergeability snapshot
The reviewed head remains the completed Phase 4 result. Main advanced after that terminal wave, so the new conflict is not an unresolved Phase 4 finding and will be handled by the composed Phase 5 mergeability gate. No code was modified or pushed while posting this advisory. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Pass the fully resolved registry URL into direct MCP integration so the persisted apm config value matches the diagnostic and behavior. Addresses Copilot inline on src/apm_cli/commands/install.py:844. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Thread user scope through target resolution, bootstrap the user manifest, reject workspace-only selections before writes, and preserve registry HTTP and credential boundaries. Adds exact lifecycle, architecture, and mutation evidence for panel follow-ups. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Validate and filter user-scope runtime capability before manifest bootstrap, preserve replay-equivalent mixed selections, and codify the contract as OpenAPM req-tg-014. Addresses review-panel and spec-guardian follow-ups. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Reject opaque registry query credentials, redact ambient URL components, publish the req-tg-014 conformance disclosure, and align global-target help and docs. Addresses final review-panel follow-ups. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep direct MCP dry-runs write-free, discover runtimes from the user deploy root, and fail closed when rendering malformed or credential-bearing ambient registry URLs. Adds real-binary target and redaction coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Apply exclusions and experimental gates before user state, pin validated ambient registry identity for replay, require explicit ambient HTTP opt-in, and align dry-run guidance. Adds real-binary regression evidence for each path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Require exact plaintext-registry opt-in, reject embedded client credentials, validate persisted ports, and align CLI recovery guidance with the tested manifest and runtime boundaries. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
6d04d36 to
4258f40
Compare

fix(mcp): honor global scope for direct MCP installs
TL;DR
apm install -g --mcp NAMEnow uses the user-scope manifest, lockfile, and runtime configuration paths instead of being rejected before target resolution. The same install-scope decision now drives both the direct MCP branch and the standard install pipeline, while runtime capability filtering still skips workspace-only targets.Note
Closes #2548.
Problem (WHY)
validate_mcp_conflictsrejected every direct MCP install that included--global, even though the CLI reference says "MCP servers deploy only to global-capable runtimes".Approach (WHAT)
InstallScopeonce ininstall()from--global.Implementation (HOW)
src/apm_cli/commands/install.py- computes one install scope before direct MCP dispatch and forwards it to user-manifest and integrator paths.src/apm_cli/install/mcp/conflicts.py- removes the obsolete blanket rejection and no longer accepts scope as conflict input.tests/unit/test_install_command.py- adds the [BUG]apm install -g --mcpis rejected outright: "MCP servers are project-scoped" #2548 regression trap proving that global direct MCP installs mutate only the user manifest and passInstallScope.USERdownstream.tests/integration/test_config_surface_lifecycle_contract.py- exercises the branch CLI end to end and verifies the Claude user config write.tests/unit/install/test_mcp_conflicts.py,tests/integration/test_wave2_adapters_coverage.py, andtests/unit/test_global_mcp_scope.py- align conflict coverage and preserve global-capable versus workspace-only runtime filtering..apm/architecture/owners/install-deployment.json,scripts/architecture_linter/checks/install_request_and_source.py, and architecture tests - register and enforce the single install-scope owner in the current sharded linter.apm-usagecommand resource, andopenapm-v0.1.md- document user-manifest routing, zero/mixed target behavior, registry safety, and req-tg-014.Diagrams
Legend: The dashed nodes are the scope-routing path changed by this PR; adapter capability filtering remains downstream.
flowchart LR subgraph Select[Scope selection] G[--global] --> S[InstallScope.USER] end subgraph Direct[Direct MCP install] S --> H[_handle_mcp_install] H --> M[user APM manifest] H --> I[MCPIntegrator] end subgraph Filter[Runtime capability filtering] I --> C[global-capable runtime] I --> W[workspace-only runtime] C --> U[user config write] W --> K[skip] end classDef new stroke-dasharray: 5 5; class S,H,M,I new;Trade-offs
InstallScope; it does not re-derive scope fromglobal_.supports_user_scopefiltering remains unchanged rather than duplicating a runtime allowlist at CLI ingress.Benefits
apm install -g --mcptransport forms now pass CLI validation.~/.apm/and deployed to user runtime config, not the current project.Validation
Targeted suites:
Real CLI lifecycle regression:
Canonical lint and boundary gates:
Mutation checks:
Scenario Evidence
apm install -g --mcp NAMEwrites the user manifest and a global-capable runtime configtests/unit/test_install_command.py::TestInstallMcpFlag::test_global_mcp_uses_user_manifest_scope(regression-trap for #2548)tests/integration/test_config_surface_lifecycle_contract.py::test_global_direct_mcp_uses_user_manifest_and_runtime_configtests/unit/test_install_command.py::TestInstallMcpFlag::test_global_mcp_rejects_workspace_only_target_before_manifest_writetests/unit/test_install_command.py::TestInstallMcpFlag::test_global_mcp_filters_mixed_target_set_before_dispatchtests/integration/test_config_surface_lifecycle_contract.py::test_global_direct_mcp_filters_mixed_and_rejects_zero_supported_targetstests/integration/test_config_surface_lifecycle_contract.py::test_configured_mcp_registry_drives_global_direct_installtests/integration/test_config_surface_lifecycle_contract.py::test_ambient_registry_source_is_pinned_for_replaytests/unit/install/test_mcp_registry_module.py::TestRegistryEnvOverride::test_http_url_preserves_separate_opt_in_when_disabledtests/unit/test_install_command.py::TestInstallMcpFlag::test_registry_env_url_pins_source_without_http_opt_intests/unit/test_install_command.py::TestInstallMcpFlag::test_invalid_configured_registry_url_fails_before_dispatchtests/unit/install/test_mcp_registry_module.py::TestValidateRegistryUrl::test_query_and_fragment_rejectedtests/integration/test_config_surface_lifecycle_contract.py::test_ambient_registry_credentials_never_reach_manifest_or_outputtests/unit/test_install_command.py::TestInstallMcpFlag::test_global_mcp_dry_run_creates_no_user_statetests/integration/test_config_surface_lifecycle_contract.py::test_global_direct_mcp_dry_run_creates_no_user_stateHow to test
~/.apm/apm.ymlwithtargets: [claude].apm install -g --mcp probe --no-policy -- echo readyand expect exit code0.probeappears in~/.apm/apm.ymland~/.claude.json, not the projectapm.yml.Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com