Harder pipeline rewire monitoring and error handling

[georgy-gorelko]

• Fixed branch policy check for pipelines with prefixed repository names to use repository ID directly when available, preventing 404 errors during rewiring
• Skip branch policy checks for disabled repositories with a warning instead of attempting API calls that may fail
• Skip pipeline rewiring entirely for disabled repositories, exiting early with an appropriate warning message
• Fixed misleading success message that appeared even when pipeline rewiring was skipped for disabled repositories
• Fixed monitor timeout minutes to only display when --dry-run mode is enabled, reducing confusion during regular pipeline rewiring operations
• Check if pipeline is disabled before attempting to queue a test build, preventing 400 Bad Request errors and providing clear warning messages

[Copilot]

Pull request overview

This PR improves error handling and monitoring for the pipeline rewiring process, with a focus on handling disabled repositories and pipelines more gracefully. The changes prevent API errors (404s and 400s) by checking repository and pipeline status before attempting operations, and fix misleading user feedback when operations are skipped.

Key Changes:

• Enhanced repository and pipeline status checking to skip operations on disabled resources with clear warnings instead of errors
• Improved branch policy checks to use repository ID directly when available, reducing API calls and preventing 404 errors
• Fixed misleading success message that appeared even when pipeline rewiring was skipped
• Refined monitor timeout logging to only display in dry-run mode

Reviewed changes

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

Show a summary per file

File	Description
src/ado2gh/Commands/RewirePipeline/RewirePipelineCommandHandler.cs	Added conditional success logging based on rewire result; added monitor timeout logging in dry-run mode; removed trailing whitespace from comment
src/ado2gh/Commands/RewirePipeline/RewirePipelineCommandArgs.cs	Added custom Log method override to exclude MonitorTimeoutMinutes and AdoPat from logging
src/ado2gh/Commands/RewirePipeline/RewirePipelineCommand.cs	Updated MonitorTimeoutMinutes option description to clarify it's for dry-run mode only; set explicit default value
src/OctoshiftCLI.Tests/ado2gh/Commands/RewirePipeline/RewirePipelineCommandHandler_ErrorHandlingTests.cs	Updated mock to return boolean for RewirePipelineToGitHub and verify success logging
src/OctoshiftCLI.Tests/Services/PipelineTestServiceTests.cs	Added IsPipelineEnabled mocks to existing tests; added comprehensive test for disabled pipeline scenario
src/OctoshiftCLI.Tests/Octoshift/Services/AdoPipelineTriggerService_ErrorHandlingTests.cs	Updated tests to verify boolean return value from RewirePipelineToGitHub; added isDisabled field to mock responses
src/OctoshiftCLI.Tests/Octoshift/Services/AdoPipelineTriggerService_BranchPolicyTests.cs	Added tests for disabled repository handling and repository ID direct usage; updated all branch policy tests with isDisabled field; added comprehensive tests for 404 scenarios
src/OctoshiftCLI.Tests/Octoshift/Services/AdoApiTests.cs	Added comprehensive tests for IsPipelineEnabled covering enabled, disabled, paused, and missing queueStatus scenarios
src/Octoshift/Services/PipelineTestService.cs	Added pipeline enabled check before attempting to queue test builds
src/Octoshift/Services/AdoPipelineTriggerService.cs	Changed RewirePipelineToGitHub to return bool; added repository disabled checks; enhanced caching to track disabled status; updated IsPipelineRequiredByBranchPolicy to accept and use repository ID directly
src/Octoshift/Services/AdoApi.cs	Added IsPipelineEnabled method to check pipeline queueStatus
src/Octoshift/Commands/CommandArgs.cs	Made Log method virtual to allow overrides
RELEASENOTES.md	Added user-facing descriptions of all fixes and improvements

Comments suppressed due to low confidence (1)

src/OctoshiftCLI.Tests/ado2gh/Commands/RewirePipeline/RewirePipelineCommandHandler_ErrorHandlingTests.cs:135

• Consider adding a test case for when RewirePipelineToGitHub returns false (e.g., when the repository is disabled). This should verify that:

1. No success message is logged
2. No error is thrown (since it's a graceful skip, not an error)
   This would ensure the conditional success logging on lines 121-124 of the handler works correctly.

        public async Task HandleRegularRewire_Should_Succeed_When_Pipeline_Found()
        {
            // Arrange
            var defaultBranch = "main";
            var clean = "true";
            var checkoutSubmodules = "false";
            var triggers = new JArray();

            _mockAdoApi.Setup(x => x.GetPipelineId(ADO_ORG, ADO_TEAM_PROJECT, ADO_PIPELINE))
                .ReturnsAsync(PIPELINE_ID);

            _mockAdoApi.Setup(x => x.GetPipeline(ADO_ORG, ADO_TEAM_PROJECT, PIPELINE_ID))
                .ReturnsAsync((defaultBranch, clean, checkoutSubmodules, triggers));

            _mockAdoPipelineTriggerService.Setup(x => x.RewirePipelineToGitHub(
                ADO_ORG, ADO_TEAM_PROJECT, PIPELINE_ID, defaultBranch, clean, checkoutSubmodules,
                GITHUB_ORG, GITHUB_REPO, SERVICE_CONNECTION_ID, triggers, null))
                .ReturnsAsync(true);

            var args = new RewirePipelineCommandArgs
            {
                AdoOrg = ADO_ORG,
                AdoTeamProject = ADO_TEAM_PROJECT,
                AdoPipeline = ADO_PIPELINE,
                GithubOrg = GITHUB_ORG,
                GithubRepo = GITHUB_REPO,
                ServiceConnectionId = SERVICE_CONNECTION_ID,
                DryRun = false
            };

            // Act
            await _handler.Handle(args);

            // Assert
            _mockAdoPipelineTriggerService.Verify(x => x.RewirePipelineToGitHub(
                ADO_ORG, ADO_TEAM_PROJECT, PIPELINE_ID, defaultBranch, clean, checkoutSubmodules,
                GITHUB_ORG, GITHUB_REPO, SERVICE_CONNECTION_ID, triggers, null), Times.Once);

            // Verify success was logged
            _mockOctoLogger.Verify(x => x.LogSuccess("Successfully rewired pipeline"), Times.Once);

            // Verify no errors were logged
            _mockOctoLogger.Verify(x => x.LogError(It.IsAny<string>()), Times.Never);
        }

[github-actions]

Unit Test Results

    1 files      1 suites   10m 24s ⏱️
1 028 tests 1 028 ✅ 0 💤 0 ❌
1 029 runs  1 029 ✅ 0 💤 0 ❌

Results for commit 39357d9.

♻️ This comment has been updated with latest results.

[brianaj]

Small comment on release notes to make a bit more clear and remove what isn't in this PR. Also I never got a response in #1436 (comment), is this PR going to fix this issue or is it still a separate issue with manually rewiring? Trying to understand why we need this fix as it seems like this was introduced with v1.19.0 that added branch policy checking and trigger preservation and if so want to make sure functional testing is done this time around please.

[github-actions]

Package	Line Rate	Branch Rate	Complexity	Health
ado2gh	71%	70%	737	➖
Octoshift	84%	74%	1796	✔
bbs2gh	83%	78%	663	✔
gei	81%	73%	608	✔
Summary	81% (7926 / 9819)	74% (1945 / 2635)	3804	✔

[brianaj]

@georgy-gorelko can you see the code scanning results https://github.com/github/gh-gei/pull/1465/checks?check_run_id=58493519789? Those 2 warnings should also be addressed.