Skip to content

Conversation

@conniey
Copy link
Member

@conniey conniey commented Dec 29, 2025

What does this PR do?

Adds support to publish public packages to 1ES repository for internal consumption. Depends on Azure/azure-sdk-tools#13221 to be merged.

GitHub issue number?

Pre-merge Checklist

  • Required for All PRs
    • Read contribution guidelines
    • PR title clearly describes the change
    • Commit history is clean with descriptive messages (cleanup guide)
    • Added comprehensive tests for new/modified functionality
    • Updated servers/Azure.Mcp.Server/CHANGELOG.md and/or servers/Fabric.Mcp.Server/CHANGELOG.md for product changes (features, bug fixes, UI/UX, updated dependencies)
  • For MCP tool changes:
    • One tool per PR: This PR adds or modifies only one MCP tool for faster review cycles
    • Updated servers/Azure.Mcp.Server/README.md and/or servers/Fabric.Mcp.Server/README.md documentation
    • Validate README.md changes using script at eng/scripts/Process-PackageReadMe.ps1. See Package README
    • Updated command list in /servers/Azure.Mcp.Server/docs/azmcp-commands.md and/or /docs/fabric-commands.md
    • Run .\eng\scripts\Update-AzCommandsMetadata.ps1 to update tool metadata in azmcp-commands.md (required for CI)
    • For new or modified tool descriptions, ran ToolDescriptionEvaluator and obtained a score of 0.4 or more and a top 3 ranking for all related test prompts
    • For tools with new names, including new tools or renamed tools, update consolidated-tools.json
    • For new tools associated with Azure services or publicly available tools/APIs/products, add URL to documentation in the PR description
  • Extra steps for Azure MCP Server tool changes:
    • Updated test prompts in /servers/Azure.Mcp.Server/docs/e2eTestPrompts.md
    • 👉 For Community (non-Microsoft team member) PRs:
      • Security review: Reviewed code for security vulnerabilities, malicious code, or suspicious activities before running tests (crypto mining, spam, data exfiltration, etc.)
      • Manual tests run: added comment /azp run mcp - pullrequest - live to run Live Test Pipeline

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds support for publishing npm packages to a 1ES (One Engineering System) internal repository alongside the existing public Azure SDK repository. The changes enable dual-publishing to both public and internal feeds during the CI/CD pipeline execution.

Key changes:

  • Enhanced the publish-to-dev-feed.yml template to support custom authentication endpoints and configurable access levels
  • Added new publish steps in release and integration jobs to publish to the 1ES repository
  • Modified the npmrc authentication step to accept custom service connection endpoints

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
eng/pipelines/templates/steps/publish-to-dev-feed.yml Added CustomEndpoint and Access parameters; refactored npm publish command to conditionally include access and tag flags
eng/pipelines/templates/jobs/npm/release-npm.yml Added new template invocation to publish packages to 1ES repository with custom endpoint
eng/pipelines/templates/jobs/integration.yml Added new template invocation to publish packages to 1ES repository with empty access parameter
eng/common/pipelines/templates/steps/create-authenticated-npmrc.yml Added customEndpoint parameter to support custom Azure DevOps service connections for authentication

parameters:
PathToArtifacts: $(Pipeline.Workspace)/packages_npm
Registry: https://pkgs.dev.azure.com/1es-mcp/1es-mcp/_packaging/1es-mcp-registry/npm/registry/
CustomEndpoint: 'Azure SDK MCP Server Connection'
Copy link

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Access parameter is set to an empty string ('') here, but in the publish-to-dev-feed.yml template, the default value for Access is 'public'. This means that when Access is explicitly set to an empty string, the condition if (![string]::IsNullOrEmpty($access)) on line 44 of publish-to-dev-feed.yml will evaluate to false (since an empty string is considered "empty"), and no --access flag will be added to the npm publish command.

However, this appears to be intentional behavior - setting Access to '' is likely meant to omit the --access flag entirely for the 1ES repository. Consider adding a comment to clarify this intentional behavior, or if this is unintentional, remove the Access parameter override to use the default 'public' value.

Suggested change
CustomEndpoint: 'Azure SDK MCP Server Connection'
CustomEndpoint: 'Azure SDK MCP Server Connection'
# Intentionally set Access to empty string for the 1ES MCP registry so that
# publish-to-dev-feed.yml does not pass an --access flag to `npm publish`.
# The template's default is 'public', but that behavior is not desired here.

Copilot uses AI. Check for mistakes.
parameters:
npmrcPath: ${{parameters.PathToArtifacts}}/.npmrc
registryUrl: ${{parameters.Registry}}
customEndpoint: ${{parameters.CustomEndpoint}}
Copy link

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parameter name uses inconsistent casing: 'CustomEndpoint' (PascalCase) is used in the parameter definition on line 8, but 'customEndpoint' (camelCase) is used when passing it to the npmAuthenticate task on line 32. While YAML template parameters in Azure Pipelines are case-insensitive, this inconsistency reduces code clarity and maintainability. Consider standardizing on one casing convention throughout the codebase.

Copilot uses AI. Check for mistakes.
@conniey conniey force-pushed the conniey/add-1es-publishing branch 3 times, most recently from daa30ea to b193bb0 Compare January 12, 2026 19:15
@conniey conniey force-pushed the conniey/add-1es-publishing branch from b193bb0 to 1cff609 Compare January 12, 2026 23:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Untriaged

Development

Successfully merging this pull request may close these issues.

3 participants