Skip to content

Commit 106aa13

Browse files
sellakumaranclaude
andcommitted
refactor: address code review issues in publish command cleanup
- Remove unused agentBlueprintService parameter from CreateCommand signature and its callsite in Program.cs (CR-001) - Replace magic-number zip cap (4) and arbitrary file padding loop with a clean LINQ expression over known candidate filenames (CR-002) - Remove dead --verbose option binding in publish command handler; startup- level --verbose in Program.cs continues to work (CR-003) - Add CHANGELOG [Unreleased] entry for the breaking behavior change (CR-004) - Remove redundant Console.SetIn call inside zip creation test; constructor already sets it (CR-005) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 85d0c85 commit 106aa13

4 files changed

Lines changed: 9 additions & 39 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
66

77
## [Unreleased]
88

9+
### Changed
10+
- `a365 publish` no longer uploads the agent automatically. It now updates manifest IDs, creates `manifest.zip`, and prints instructions for manually uploading via Microsoft 365 Admin Center (Agents > All agents > Upload custom agent).
11+
912
### Fixed
1013
- macOS/Linux: device code fallback when browser authentication is unavailable (#309)
1114
- Linux: MSAL fallback when PowerShell `Connect-MgGraph` fails in non-TTY environments (#309)

src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommand.cs

Lines changed: 5 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -60,18 +60,13 @@ private static string GetProjectDirectory(Agent365Config config, ILogger logger)
6060
public static Command CreateCommand(
6161
ILogger<PublishCommand> logger,
6262
IConfigService configService,
63-
AgentBlueprintService agentBlueprintService,
6463
ManifestTemplateService manifestTemplateService)
6564
{
6665
var command = new Command("publish", "Update manifest.json IDs and create a manifest package for upload to the Microsoft 365 Admin Center");
6766

6867
var dryRunOption = new Option<bool>("--dry-run", "Show changes without writing file or calling APIs");
69-
var verboseOption = new Option<bool>(
70-
["--verbose", "-v"],
71-
description: "Enable verbose logging");
7268

7369
command.AddOption(dryRunOption);
74-
command.AddOption(verboseOption);
7570

7671
command.SetHandler(async (System.CommandLine.Invocation.InvocationContext context) =>
7772
{
@@ -217,25 +212,12 @@ public static Command CreateCommand(
217212
try { File.Delete(zipPath); } catch { /* ignore */ }
218213
}
219214

220-
// Identify files to include in zip; agenticUserTemplateManifest.json is explicitly listed
221-
// to ensure it is always included regardless of other files present in the directory
222-
var expectedFiles = new List<string>();
215+
// Collect all known manifest files that exist; order is deterministic
223216
string[] candidateNames = ["manifest.json", "agenticUserTemplateManifest.json", "color.png", "outline.png", "logo.png", "icon.png"];
224-
foreach (var name in candidateNames)
225-
{
226-
var p = Path.Combine(manifestDir, name);
227-
if (File.Exists(p)) expectedFiles.Add(p);
228-
if (expectedFiles.Count == 4) break;
229-
}
230-
// If still fewer than 4, add any other files to reach 4 (non recursive)
231-
if (expectedFiles.Count < 4)
232-
{
233-
foreach (var f in Directory.EnumerateFiles(manifestDir).Where(f => !expectedFiles.Contains(f)))
234-
{
235-
expectedFiles.Add(f);
236-
if (expectedFiles.Count == 4) break;
237-
}
238-
}
217+
var expectedFiles = candidateNames
218+
.Select(name => Path.Combine(manifestDir, name))
219+
.Where(File.Exists)
220+
.ToList();
239221

240222
if (expectedFiles.Count == 0)
241223
{

src/Microsoft.Agents.A365.DevTools.Cli/Program.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ static async Task<int> Main(string[] args)
125125
rootCommand.AddCommand(ConfigCommand.CreateCommand(configLogger, wizardService: wizardService, clientAppValidator: clientAppValidator));
126126
rootCommand.AddCommand(QueryEntraCommand.CreateCommand(queryEntraLogger, configService, executor, graphApiService, agentBlueprintService));
127127
rootCommand.AddCommand(CleanupCommand.CreateCommand(cleanupLogger, configService, botConfigurator, executor, agentBlueprintService, confirmationProvider, federatedCredentialService));
128-
rootCommand.AddCommand(PublishCommand.CreateCommand(publishLogger, configService, agentBlueprintService, manifestTemplateService));
128+
rootCommand.AddCommand(PublishCommand.CreateCommand(publishLogger, configService, manifestTemplateService));
129129

130130
// Wrap all command handlers with exception handling
131131
// Build with middleware for global exception handling

src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandTests.cs

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ public class PublishCommandTests : IDisposable
2828
{
2929
private readonly ILogger<PublishCommand> _logger;
3030
private readonly IConfigService _configService;
31-
private readonly AgentBlueprintService _blueprintService;
3231
private readonly ManifestTemplateService _manifestTemplateService;
3332
private readonly TextReader _originalConsoleIn = Console.In;
3433

@@ -37,13 +36,6 @@ public PublishCommandTests()
3736
_logger = Substitute.For<ILogger<PublishCommand>>();
3837
_configService = Substitute.For<IConfigService>();
3938

40-
var graphApiService = Substitute.ForPartsOf<GraphApiService>();
41-
42-
// AgentBlueprintService needs (ILogger, GraphApiService)
43-
_blueprintService = Substitute.ForPartsOf<AgentBlueprintService>(
44-
Substitute.For<ILogger<AgentBlueprintService>>(),
45-
graphApiService);
46-
4739
// ManifestTemplateService needs only ILogger
4840
_manifestTemplateService = Substitute.ForPartsOf<ManifestTemplateService>(
4941
Substitute.For<ILogger<ManifestTemplateService>>());
@@ -71,7 +63,6 @@ public async Task PublishCommand_WithMissingBlueprintId_ShouldReturnExitCode1()
7163
var command = PublishCommand.CreateCommand(
7264
_logger,
7365
_configService,
74-
_blueprintService,
7566
_manifestTemplateService);
7667

7768
var root = new RootCommand();
@@ -120,7 +111,6 @@ public async Task PublishCommand_WithDryRun_ShouldReturnExitCode0()
120111
var command = PublishCommand.CreateCommand(
121112
_logger,
122113
_configService,
123-
_blueprintService,
124114
_manifestTemplateService);
125115

126116
var root = new RootCommand();
@@ -173,13 +163,9 @@ public async Task PublishCommand_WithValidConfig_CreatesZipAndReturnsExitCode0()
173163
};
174164
_configService.LoadAsync().Returns(config);
175165

176-
// Redirect stdin so interactive prompts auto-answer
177-
Console.SetIn(new StringReader("n\n\n"));
178-
179166
var command = PublishCommand.CreateCommand(
180167
_logger,
181168
_configService,
182-
_blueprintService,
183169
_manifestTemplateService);
184170

185171
var root = new RootCommand();
@@ -211,7 +197,6 @@ public async Task PublishCommand_WithException_ShouldReturnExitCode1()
211197
var command = PublishCommand.CreateCommand(
212198
_logger,
213199
_configService,
214-
_blueprintService,
215200
_manifestTemplateService);
216201

217202
var root = new RootCommand();

0 commit comments

Comments
 (0)