Skip to content

Fix CustomizeUpdate tool for reporting correct operation status#14491

Merged
samvaity merged 5 commits intomainfrom
cleanup
Mar 19, 2026
Merged

Fix CustomizeUpdate tool for reporting correct operation status#14491
samvaity merged 5 commits intomainfrom
cleanup

Conversation

@samvaity
Copy link
Member

@samvaity samvaity commented Mar 12, 2026

flowchart TD
    Start([HandleCommand / UpdateAsync]) --> Validate{Validate Inputs}
    Validate -->|Invalid| ErrInput[Return InvalidInput Error]
    Validate -->|Valid| GetLang[Get LanguageService]
    GetLang --> GetFeedback[GetFeedbackItems]
    GetFeedback -->|No items| ErrNoFeedback[Return InvalidInput Error]
    GetFeedback -->|Has items| Pass1["Pass 1: ClassifyItemsAsync"]

    Pass1 -->|No classifications| ErrClassify[Return Classification Error]
    Pass1 -->|Has classifications| Loop1["Loop: Process Each Classification"]

    subgraph ClassifyFeedback["Pass 1: Classify Feedbacks"]
        direction TB
        Pass1
        ErrClassify
        Loop1
        Loop1 -->|TSP_APPLICABLE| ApplyTSP["ApplyCustomizationAsync\n(TypeSpec changes)"]
        Loop1 -->|CODE_CUSTOMIZATION| TrackCode[Mark for code patching]
        Loop1 -->|REQUIRES_MANUAL| TrackManual[Add to manualInterventions]
        Loop1 -->|SUCCESS| TrackNoChange["Remove – no change\nneeded"]
        ApplyTSP -->|Success| TrackTSPOk[Track changesMade]
        ApplyTSP -->|Failure| TrackTSPFail[Track failure reason]
    end

    TrackTSPOk --> EarlyExit
    TrackTSPFail --> EarlyExit
    TrackCode --> EarlyExit
    TrackManual --> EarlyExit
    TrackNoChange --> EarlyExit

    EarlyExit{Early exit checks}
    EarlyExit -->|"All manual..."| RetManual[Return:\nManualInterventionRequired]
    EarlyExit -->|"All success"| RetNoChange[Return: No changes needed]
    EarlyExit -->|"TSP fixes applied"| RegenGate{tspFixSucceeded > 0?}

    subgraph RegenBuild["Regen + Build after TSP"]
        direction TB
        RegenGate
        RegenGate -->|Yes| Regen["UpdateGenerationAsync"]
        Regen -->|Success| Build1["BuildAsync"]
        Regen -->|Failed| EnrichRegen[Enrich items with regen error]
        Build1 -->|"Pass & no code\ncustomizations"| RetBuildOk1["Return: Success – TSP only"]
        Build1 -->|Fail| EnrichBuild[Enrich items with build error]
    end

    RegenGate -->|No| Pass2Gate
    EnrichRegen --> Pass2Gate
    EnrichBuild --> Pass2Gate

    subgraph Pass2["Pass 2: Re-classify"]
        direction TB
        Pass2Gate{Remaining items?}
        Pass2Gate -->|Yes| Reclass["ClassifyItemsAsync – 2nd\npass"]
        Reclass -->|CODE_CUSTOMIZATION| MoreCode[codeCustomizations++]
        Reclass -->|REQUIRES_MANUAL| MoreManual[manualInterventions++]
        Reclass -->|SUCCESS| MoreSuccess[Remove item]
    end

    Pass2Gate -->|No| NeedBuild
    MoreCode --> NeedBuild
    MoreManual --> NeedBuild
    MoreSuccess --> NeedBuild

    NeedBuild{"Build not yet run\nor failed?"}
    NeedBuild -->|"No build yet"| Build2["BuildAsync\n(for error context)"]
    Build2 --> CheckBuild2
    NeedBuild -->|"Already built"| CheckBuild2

    CheckBuild2{"Build passed &\nno code customizations?"}
    CheckBuild2 -->|Yes| RetBuildOk2[Return Success +\nTypeSpec changes]
    CheckBuild2 -->|No| SupportCheck{IsCustomizedCodeUpdate\nSupported?}

    SupportCheck -->|No| RetNoLang[Return NoLanguageService Error]
    SupportCheck -->|Yes| HasCustom{HasCustomizations?}

    HasCustom -->|No files| RetNoCust[Return BuildNoCustomizationsFailed]
    HasCustom -->|Has files| Patch["ApplyPatchesAsync\n(code customization patches)"]

    Patch -->|No patches| RetNoPatch[Return PatchesFailed Error]
    Patch -->|Has patches| JavaCheck{Language == Java?}

    JavaCheck -->|Yes| RegenJava["UpdateGenerationAsync\n(Java post-patch regen)"]
    RegenJava -->|Failed| RetRegenFail[Return RegenerateAfterPatchesFailed]
    RegenJava -->|Success| FinalBuild

    JavaCheck -->|No| FinalBuild["Final BuildAsync"]

    FinalBuild -->|Pass| RetFinalOk[Return Success +\npatches + changes]
    FinalBuild -->|Fail| RetFinalFail[Return BuildAfterPatchesFailed]

    style Start fill:#4a90d9,color:#fff
    style RetBuildOk1 fill:#27ae60,color:#fff
    style RetBuildOk2 fill:#27ae60,color:#fff
    style RetFinalOk fill:#27ae60,color:#fff
    style RetNoChange fill:#27ae60,color:#fff
    style RetManual fill:#f39c12,color:#fff
    style ErrInput fill:#e74c3c,color:#fff
    style ErrNoFeedback fill:#e74c3c,color:#fff
    style ErrClassify fill:#e74c3c,color:#fff
    style RetNoLang fill:#e74c3c,color:#fff
    style RetNoCust fill:#e74c3c,color:#fff
    style RetNoPatch fill:#e74c3c,color:#fff
    style RetRegenFail fill:#e74c3c,color:#fff
    style RetFinalFail fill:#e74c3c,color:#fff
    style ClassifyFeedback fill:#fef9e7,stroke:#f0e68c
    style RegenBuild fill:#fef9e7,stroke:#f0e68c
    style Pass2 fill:#fef9e7,stroke:#f0e68c
Loading

@github-actions github-actions bot added the azsdk-cli Issues related to Azure/azure-sdk-tools::tools/azsdk-cli label Mar 12, 2026
@samvaity samvaity self-assigned this Mar 12, 2026
@samvaity samvaity added the Scenario 2 Used for tracking work related to the Dev Inner Loop group's Scenario 2 spec. label Mar 12, 2026
@samvaity samvaity marked this pull request as ready for review March 12, 2026 21:00
@samvaity samvaity requested a review from a team as a code owner March 12, 2026 21:00
Copilot AI review requested due to automatic review settings March 12, 2026 21:00
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 updates the CustomizedCodeUpdateTool workflow to correctly report failed operations via the standard CommandResponse status/error fields, improving downstream consumers’ ability to detect failure conditions.

Changes:

  • Populate ResponseError for CustomizedCodeUpdateResponse failure cases so OperationStatus/ExitCode reflect failures correctly.
  • Adjust build execution within the retry loop to avoid unnecessary builds in some iterations.
  • Update response formatting/documentation around BuildResult (including printing build output in formatted output when present).

Reviewed changes

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

File Description
tools/azsdk-cli/Azure.Sdk.Tools.Cli/Tools/TypeSpec/CustomizedCodeUpdateTool.cs Sets ResponseError on failure responses and tweaks build/regen flow in the retry loop and patch validation paths.
tools/azsdk-cli/Azure.Sdk.Tools.Cli/Models/Responses/Package/CustomizedCodeUpdateResponse.cs Updates BuildResult docstring and includes BuildResult in formatted output.
Comments suppressed due to low confidence (2)

tools/azsdk-cli/Azure.Sdk.Tools.Cli/Tools/TypeSpec/CustomizedCodeUpdateTool.cs:155

  • The PR’s main behavior change is setting ResponseError when Success = false so OperationStatus is reported correctly. The existing unit tests for failure cases assert Success/ErrorCode/BuildResult, but none assert that ResponseError is set (and therefore that OperationStatus/ExitCode flips to failed). Adding assertions for ResponseError in a couple representative failure tests would prevent regressions in the status-reporting behavior this PR is fixing.
            return new CustomizedCodeUpdateResponse
            {
                Success = false,
                ResponseError = $"Package path does not exist: {packagePath}",
                Message = $"Package path does not exist: {packagePath}",
                ErrorCode = CustomizedCodeUpdateResponse.KnownErrorCodes.InvalidInput,
                BuildResult = $"Package path does not exist: {packagePath}"
            };

tools/azsdk-cli/Azure.Sdk.Tools.Cli/Models/Responses/Package/CustomizedCodeUpdateResponse.cs:36

  • The updated BuildResult XML comment says it’s “Raw build output” and is set on both success and failure, but the tool is populating it with LanguageService.BuildAsync’s ErrorMessage, which is explicitly null on success. Either update the documentation to reflect that this is build failure output/error context, or change the implementation to actually capture and populate successful build output.
    /// <summary>
    /// Raw build output. Set on both success and failure to provide context in MCP responses.
    /// </summary>
    [JsonPropertyName("buildResult")]
    [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
    public string? BuildResult { get; set; }

You can also share your feedback on Copilot code review. Take the survey.

@samvaity samvaity requested a review from m-redding March 12, 2026 22:06
Copy link
Member

@m-redding m-redding left a comment

Choose a reason for hiding this comment

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

lgtm just left a couple notes, could be tweaked in a later PR too

…fixes and builds, pass 2 re-evaluates with error context
Copy link
Member

@m-redding m-redding left a comment

Choose a reason for hiding this comment

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

Looks good! I think the new flow makes so much more sense!! Just had a few optional nits

…cleanup

# Conflicts:
#	tools/azsdk-cli/Azure.Sdk.Tools.Cli/Tools/TypeSpec/CustomizedCodeUpdateTool.cs
@samvaity samvaity merged commit c446918 into main Mar 19, 2026
13 checks passed
@samvaity samvaity deleted the cleanup branch March 19, 2026 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

azsdk-cli Issues related to Azure/azure-sdk-tools::tools/azsdk-cli Scenario 2 Used for tracking work related to the Dev Inner Loop group's Scenario 2 spec.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants