From ac4414f08a503b6a8972de6d5a79d32a2ed99a15 Mon Sep 17 00:00:00 2001 From: Sameeksha Vaity Date: Thu, 11 Sep 2025 15:14:57 -0700 Subject: [PATCH] Use shared tsp-client for regenerate func --- .../TypeSpec/TspClientUpdateToolTests.cs | 28 ++++-- .../Helpers/ITspClientHelper.cs | 12 ++- .../Helpers/TspClientHelper.cs | 33 +++++++ .../Responses/TspClientUpdateResponse.cs | 7 +- .../Tools/TypeSpec/TspClientUpdateTool.cs | 97 +++++++++++++------ 5 files changed, 134 insertions(+), 43 deletions(-) diff --git a/tools/azsdk-cli/Azure.Sdk.Tools.Cli.Tests/Tools/TypeSpec/TspClientUpdateToolTests.cs b/tools/azsdk-cli/Azure.Sdk.Tools.Cli.Tests/Tools/TypeSpec/TspClientUpdateToolTests.cs index c1f5c7bc091..e7032532d79 100644 --- a/tools/azsdk-cli/Azure.Sdk.Tools.Cli.Tests/Tools/TypeSpec/TspClientUpdateToolTests.cs +++ b/tools/azsdk-cli/Azure.Sdk.Tools.Cli.Tests/Tools/TypeSpec/TspClientUpdateToolTests.cs @@ -3,8 +3,7 @@ using Azure.Sdk.Tools.Cli.Tools; using Microsoft.Extensions.Logging.Abstractions; using Azure.Sdk.Tools.Cli.Helpers; -using System; -using System.IO; +using Azure.Sdk.Tools.Cli.Models.Responses; namespace Azure.Sdk.Tools.Cli.Tests.Tools.CustomizedCodeUpdateTool; @@ -63,9 +62,10 @@ public async Task Auto_NoChanges_TerminatesAtValidation() { var svc = new MockNoChangeLanguageService(); var resolver = new SingleResolver(svc); - var tool = new TspClientUpdateTool(new NullLogger(), new NullOutputService(), resolver); + var tsp = new MockTspHelper(); + var tool = new TspClientUpdateTool(new NullLogger(), new NullOutputService(), resolver, tsp); var pkg = CreateTempPackageDir(); - var run = await tool.UpdateAsync("placeholder.tsp", packagePath: pkg, ct: CancellationToken.None); + var run = await tool.UpdateAsync("0123456789abcdef0123456789abcdef01234567", packagePath: pkg, ct: CancellationToken.None); Assert.That(run.Session, Is.Not.Null, "Session should be created"); Assert.That(run.Session!.LastStage, Is.EqualTo(UpdateStage.Validated), "No changes now proceed through validation"); // Slim model: no stored API change count; reaching Validated implies no changes or all handled. @@ -76,9 +76,10 @@ public async Task Auto_WithChanges_Validated() { var svc = new MockChangeLanguageService(); var resolver = new SingleResolver(svc); - var tool = new TspClientUpdateTool(new NullLogger(), new NullOutputService(), resolver); + var tsp = new MockTspHelper(); + var tool = new TspClientUpdateTool(new NullLogger(), new NullOutputService(), resolver, tsp); var pkg = CreateTempPackageDir(); - var first = await tool.UpdateAsync("placeholder-change.tsp", packagePath: pkg, ct: CancellationToken.None); + var first = await tool.UpdateAsync("89abcdef0123456789abcdef0123456789abcdef", packagePath: pkg, ct: CancellationToken.None); Assert.That(first.Session, Is.Not.Null); Assert.That(first.Session.LastStage, Is.EqualTo(UpdateStage.Validated), "Single-pass should reach validated"); } @@ -86,11 +87,12 @@ public async Task Auto_WithChanges_Validated() [Test] public async Task Validation_Failure_Then_AutoFixes_Applied() { - var tool = new TspClientUpdateTool(new NullLogger(), new NullOutputService(), new SingleResolver(new MockNoChangeLanguageService())); + var tsp = new MockTspHelper(); + var tool = new TspClientUpdateTool(new NullLogger(), new NullOutputService(), new SingleResolver(new MockNoChangeLanguageService()), tsp); int calls = 0; var svc = new TestLanguageServiceFailThenFix(() => calls++); - tool = new TspClientUpdateTool(new NullLogger(), new NullOutputService(), new SingleResolver(svc)); + tool = new TspClientUpdateTool(new NullLogger(), new NullOutputService(), new SingleResolver(svc), tsp); var pkg = CreateTempPackageDir(); - var resp = await tool.UpdateAsync("spec.tsp", packagePath: pkg, ct: CancellationToken.None); + var resp = await tool.UpdateAsync("fedcba9876543210fedcba9876543210fedcba98", packagePath: pkg, ct: CancellationToken.None); Assert.That(resp.Session, Is.Not.Null); Assert.That(resp.Session!.LastStage, Is.EqualTo(UpdateStage.Validated)); Assert.That(resp.Session.RequiresManualIntervention, Is.False); @@ -129,3 +131,11 @@ private class SingleResolver : IClientUpdateLanguageServiceResolver public Task ResolveAsync(string? packagePath, CancellationToken ct = default) => Task.FromResult(_svc); } } + +internal class MockTspHelper : ITspClientHelper +{ + public Task ConvertSwaggerAsync(string swaggerReadmePath, string outputDirectory, bool isArm, bool fullyCompatible, bool isCli, CancellationToken ct) + => Task.FromResult(new TspToolResponse { IsSuccessful = true, TypeSpecProjectPath = outputDirectory }); + public Task UpdateGenerationAsync(string tspLocationPath, string outputDirectory, bool isCli, CancellationToken ct) + => Task.FromResult(new TspToolResponse { IsSuccessful = true, TypeSpecProjectPath = outputDirectory }); +} diff --git a/tools/azsdk-cli/Azure.Sdk.Tools.Cli/Helpers/ITspClientHelper.cs b/tools/azsdk-cli/Azure.Sdk.Tools.Cli/Helpers/ITspClientHelper.cs index 4a482ab86ce..14e8ee292bb 100644 --- a/tools/azsdk-cli/Azure.Sdk.Tools.Cli/Helpers/ITspClientHelper.cs +++ b/tools/azsdk-cli/Azure.Sdk.Tools.Cli/Helpers/ITspClientHelper.cs @@ -5,9 +5,9 @@ namespace Azure.Sdk.Tools.Cli.Helpers; /// -/// Abstraction for running common tsp-client commands (convert, update, diff, map, etc.). +/// Abstraction for running common tsp-client commands (convert, update, generate, init). /// This centralizes npx invocation logic so multiple tools (Generate, Build, Update workflows) -/// can share a single implementation without instantiating each other. +/// can share a single implementation without duplicating code. /// public interface ITspClientHelper { @@ -15,4 +15,12 @@ public interface ITspClientHelper /// Runs `tsp-client convert --swagger-readme --output-dir ` with optional flags. /// Task ConvertSwaggerAsync(string swaggerReadmePath, string outputDirectory, bool isArm, bool fullyCompatible, bool isCli, CancellationToken ct); + + /// + /// Runs `tsp-client update` to regenerate a TypeSpec client into the specified output directory. + /// + /// Path to the tsp-location.yaml file. + /// Directory to place regenerated output (created if missing, must be empty or created new). + /// True when invoked from CLI flow (suppresses duplicate streamed output in error text). + Task UpdateGenerationAsync(string tspLocationPath, string outputDirectory, bool isCli, CancellationToken ct); } diff --git a/tools/azsdk-cli/Azure.Sdk.Tools.Cli/Helpers/TspClientHelper.cs b/tools/azsdk-cli/Azure.Sdk.Tools.Cli/Helpers/TspClientHelper.cs index 8c7e3d22a2c..8ce16e4e397 100644 --- a/tools/azsdk-cli/Azure.Sdk.Tools.Cli/Helpers/TspClientHelper.cs +++ b/tools/azsdk-cli/Azure.Sdk.Tools.Cli/Helpers/TspClientHelper.cs @@ -54,4 +54,37 @@ public async Task ConvertSwaggerAsync(string swaggerReadmePath, TypeSpecProjectPath = outputDirectory }; } + + public async Task UpdateGenerationAsync(string tspLocationPath, string outputDirectory, bool isCli, CancellationToken ct) + { + logger.LogInformation("tsp-client update (tsp-location): {loc} -> {out}", tspLocationPath, outputDirectory); + if (!File.Exists(tspLocationPath)) + { + return new TspToolResponse { ResponseError = $"tsp-location.yaml not found at path: {tspLocationPath}" }; + } + var workingDir = Path.GetDirectoryName(Path.GetFullPath(tspLocationPath))!; + var npxOptions = new NpxOptions( + "@azure-tools/typespec-client-generator-cli", + ["tsp-client", "update"], + logOutputStream: true, + workingDirectory: workingDir + ); + + var result = await npxHelper.Run(npxOptions, ct); + if (result.ExitCode != 0) + { + return new TspToolResponse + { + ResponseError = isCli + ? "Failed to regenerate TypeSpec client, see details in the above logs." + : "Failed to regenerate TypeSpec client, see generator output below" + Environment.NewLine + result.Output + }; + } + + return new TspToolResponse + { + IsSuccessful = true, + TypeSpecProjectPath = outputDirectory + }; + } } diff --git a/tools/azsdk-cli/Azure.Sdk.Tools.Cli/Models/Responses/TspClientUpdateResponse.cs b/tools/azsdk-cli/Azure.Sdk.Tools.Cli/Models/Responses/TspClientUpdateResponse.cs index 98b7ad36198..f76da91da0f 100644 --- a/tools/azsdk-cli/Azure.Sdk.Tools.Cli/Models/Responses/TspClientUpdateResponse.cs +++ b/tools/azsdk-cli/Azure.Sdk.Tools.Cli/Models/Responses/TspClientUpdateResponse.cs @@ -70,7 +70,12 @@ public enum UpdateStage /// /// Post-apply validation (build / tests / lint) executed and results captured. /// - Validated + Validated, + + /// + /// Update failed due to an unknown error. + /// + Failed } public class ClientUpdateSessionState diff --git a/tools/azsdk-cli/Azure.Sdk.Tools.Cli/Tools/TypeSpec/TspClientUpdateTool.cs b/tools/azsdk-cli/Azure.Sdk.Tools.Cli/Tools/TypeSpec/TspClientUpdateTool.cs index 7d23a7e00e0..4bc1aef6516 100644 --- a/tools/azsdk-cli/Azure.Sdk.Tools.Cli/Tools/TypeSpec/TspClientUpdateTool.cs +++ b/tools/azsdk-cli/Azure.Sdk.Tools.Cli/Tools/TypeSpec/TspClientUpdateTool.cs @@ -18,14 +18,16 @@ public class TspClientUpdateTool : MCPTool private readonly ILogger logger; private readonly IOutputHelper output; private readonly IClientUpdateLanguageServiceResolver languageServiceResolver; - private readonly Argument specPathArg = new(name: "spec-path", description: "Path to the .tsp specification file") { Arity = ArgumentArity.ExactlyOne }; + private readonly ITspClientHelper tspClientHelper; + private readonly Argument updateCommitSha = new(name: "update-commit-sha", description: "SHA of the commit to apply update changes for") { Arity = ArgumentArity.ExactlyOne }; private readonly Option newGenOpt = new(["--new-gen"], () => "./tmpgen", "Directory for regenerated TypeSpec output (optional)"); - public TspClientUpdateTool(ILogger logger, IOutputHelper output, IClientUpdateLanguageServiceResolver languageServiceResolver) + public TspClientUpdateTool(ILogger logger, IOutputHelper output, IClientUpdateLanguageServiceResolver languageServiceResolver, ITspClientHelper tspClientHelper) { this.logger = logger; this.output = output; this.languageServiceResolver = languageServiceResolver; + this.tspClientHelper = tspClientHelper; CommandHierarchy = [ SharedCommandGroups.TypeSpec ]; } @@ -33,7 +35,7 @@ public override Command GetCommand() { var cmd = new Command("customized-update", description: "Update customized TypeSpec-generated client code. Runs the full pipeline by default: regenerate -> diff -> map -> propose -> apply"); - cmd.AddArgument(specPathArg); + cmd.AddArgument(updateCommitSha); cmd.AddOption(SharedOptions.PackagePath); cmd.AddOption(newGenOpt); cmd.SetHandler(async ctx => await HandleUpdate(ctx, ctx.GetCancellationToken())); @@ -43,43 +45,31 @@ public override Command GetCommand() public override Task HandleCommand(InvocationContext ctx, CancellationToken ct) => Task.CompletedTask; [McpServerTool(Name = "azsdk_tsp_update"), Description("Update customized TypeSpec-generated client code")] - public async Task UpdateAsync(string specPath, string packagePath, CancellationToken ct = default) + public Task UpdateAsync(string commitSha, string packagePath, CancellationToken ct = default) + => RunUpdateAsync(commitSha, packagePath, newGenPath: null, ct); + + private async Task RunUpdateAsync(string commitSha, string packagePath, string? newGenPath, CancellationToken ct) { try { - logger.LogInformation($"Starting client update for package at: {packagePath}"); + logger.LogInformation("Starting client update for package at: {packagePath} (regenDir: {regenDir})", packagePath, newGenPath); if (!Directory.Exists(packagePath)) { SetFailure(1); - return new TspClientUpdateResponse - { - ErrorCode = "1", - ResponseError = $"Package path does not exist: {packagePath}", - Message = "" - }; + return new TspClientUpdateResponse { ErrorCode = "1", ResponseError = $"Package path does not exist: {packagePath}" }; } - if (string.IsNullOrWhiteSpace(specPath)) + if (string.IsNullOrWhiteSpace(commitSha)) { SetFailure(1); - return new TspClientUpdateResponse - { - ErrorCode = "1", - ResponseError = $"Spec path is required.", - Message = "" - }; + return new TspClientUpdateResponse { ErrorCode = "1", ResponseError = "Commit SHA is required." }; } var resolved = await languageServiceResolver.ResolveAsync(packagePath, ct); if (resolved == null) { SetFailure(1); - return new TspClientUpdateResponse - { - ErrorCode = "NoLanguageService", - ResponseError = "Could not resolve a client update language service.", - Message = "" - }; + return new TspClientUpdateResponse { ErrorCode = "NoLanguageService", ResponseError = "Could not resolve a client update language service." }; } - return await UpdateCoreAsync(specPath, packagePath, resolved, ct); + return await UpdateCoreAsync(commitSha, packagePath, resolved, ct, newGenPath); } catch (Exception ex) { @@ -88,12 +78,42 @@ public async Task UpdateAsync(string specPath, string p } } - private async Task UpdateCoreAsync(string specPath, string packagePath, IClientUpdateLanguageService languageService, CancellationToken ct) + private async Task UpdateCoreAsync(string commitSha, string packagePath, IClientUpdateLanguageService languageService, CancellationToken ct, string? newGenPath) { - var session = new ClientUpdateSessionState { SpecPath = specPath }; + var session = new ClientUpdateSessionState { SpecPath = commitSha }; + + // Determine output directory for new generation: use provided newGenPath (CLI option) or fallback. + var regenDir = ResolveRegenDirectory(packagePath, newGenPath); + if (!Directory.Exists(regenDir)) + { + Directory.CreateDirectory(regenDir); + } + session.NewGeneratedPath = regenDir; + + // Locate the existing tsp-location.yaml file within the provided packagePath and overwrite the commit: value with the new sha + var tspLocationPath = Path.Combine(packagePath, "tsp-location.yaml"); + if (File.Exists(tspLocationPath)) + { + var tspLocationContent = await File.ReadAllTextAsync(tspLocationPath, ct); + tspLocationContent = tspLocationContent.Replace("commit: ", $"commit: {commitSha}"); + await File.WriteAllTextAsync(tspLocationPath, tspLocationContent, ct); + } - // Regenerate (placeholder) + // Invoke tsp-client update + var regenResult = await tspClientHelper.UpdateGenerationAsync(tspLocationPath, regenDir, isCli: false, ct); + if (!regenResult.IsSuccessful) + { + SetFailure(1); + session.LastStage = UpdateStage.Failed; + return new TspClientUpdateResponse + { + Session = session, + ErrorCode = "RegenerateFailed", + ResponseError = regenResult.ResponseError + }; + } session.LastStage = UpdateStage.Regenerated; + // Now after regeneration, we have old generated at packagePath, new generation at regenDir to perform a diff var apiChanges = await languageService.DiffAsync(packagePath, session.NewGeneratedPath); session.LastStage = UpdateStage.Diffed; @@ -154,12 +174,13 @@ private static async Task ValidateWithAutoFixAsync(Clie private async Task HandleUpdate(InvocationContext ctx, CancellationToken ct) { - var spec = ctx.ParseResult.GetValueForArgument(specPathArg); + var spec = ctx.ParseResult.GetValueForArgument(updateCommitSha); var packagePath = ctx.ParseResult.GetValueForOption(SharedOptions.PackagePath); + var newGenPath = ctx.ParseResult.GetValueForOption(newGenOpt); try { - logger.LogInformation($"Starting client update for package at: {packagePath}"); - var resp = await UpdateAsync(spec, packagePath, ct); + logger.LogInformation("Starting client update (CLI) for package at: {packagePath} with new-gen: {newGenPath}", packagePath, newGenPath); + var resp = await RunUpdateAsync(spec, packagePath, newGenPath, ct); output.Output(resp); } catch (Exception ex) @@ -168,4 +189,18 @@ private async Task HandleUpdate(InvocationContext ctx, CancellationToken ct) output.OutputError(new TspClientUpdateResponse { ResponseError = ex.Message, ErrorCode = "ClientUpdateFailed" }); } } + + private static string ResolveRegenDirectory(string packagePath, string? newGenPath) + { + if (string.IsNullOrWhiteSpace(newGenPath)) + { + return Path.Combine(packagePath, "_generated-new"); + } + // If user supplied a relative path, place it under the package path for isolation. + if (!Path.IsPathRooted(newGenPath)) + { + return Path.GetFullPath(Path.Combine(packagePath, newGenPath)); + } + return Path.GetFullPath(newGenPath); + } }