fix(installer): read the persisted MCP port on update instead of the wizard default

RegisterMcpStep built the registration URL from ctx.ExternalMcpPort, which the
Update pipeline never repopulates from the existing installation, so any
update silently re-registered the wizard default (47822) even when
worker.config.json had a different port configured. InstallerWorkerConfig was
also missing external_mcp_port entirely, so the installer had no way to read
it back. Port 0 (external listener disabled) now skips registration instead
of pointing Claude at 127.0.0.1:0/mcp.
This commit is contained in:
mika kuns
2026-08-06 11:04:38 +02:00
parent 0f187d8e82
commit 3462ca1355
3 changed files with 88 additions and 1 deletions
@@ -52,6 +52,9 @@ public sealed class InstallerWorkerConfig
[JsonPropertyName("claude_bin")]
public string ClaudeBin { get; set; } = "claude";
[JsonPropertyName("external_mcp_port")]
public int ExternalMcpPort { get; set; } = 47_822;
private static readonly JsonSerializerOptions ReadOpts = new()
{
ReadCommentHandling = JsonCommentHandling.Skip,
@@ -5,9 +5,23 @@ namespace ClaudeDo.Installer.Steps;
public sealed class RegisterMcpStep : IInstallStep
{
private const string ServerName = "claudedo";
private readonly Func<InstallerWorkerConfig> _loadWorkerConfig;
public RegisterMcpStep(Func<InstallerWorkerConfig>? loadWorkerConfig = null)
{
_loadWorkerConfig = loadWorkerConfig ?? InstallerWorkerConfig.Load;
}
public string Name => "Register MCP with Claude";
// Resolves the URL from the persisted worker.config.json rather than ctx.ExternalMcpPort:
// an Update run never repopulates ctx from the existing installation, so ctx would still
// hold the wizard default (47822) even when the installed config has a different port.
// Returns null when the port is 0 — WorkerConfig treats that as "external listener off",
// so registering a URL against it would just point Claude at nothing.
public static string? ResolveUrl(InstallerWorkerConfig cfg) =>
cfg.ExternalMcpPort == 0 ? null : $"http://127.0.0.1:{cfg.ExternalMcpPort}/mcp";
public async Task<StepResult> ExecuteAsync(InstallContext ctx, IProgress<string> progress, CancellationToken ct)
{
if (!ctx.RegisterMcpWithClaude)
@@ -16,7 +30,12 @@ public sealed class RegisterMcpStep : IInstallStep
return StepResult.Ok();
}
var url = $"http://127.0.0.1:{ctx.ExternalMcpPort}/mcp";
var url = ResolveUrl(_loadWorkerConfig());
if (url is null)
{
progress.Report("Skipped (external_mcp_port is 0 — the external MCP listener is disabled).");
return StepResult.Ok();
}
// Drop any prior registration first so a re-run (e.g. update, changed port)
// overwrites cleanly instead of erroring on a duplicate name.
@@ -0,0 +1,65 @@
using ClaudeDo.Installer.Core;
using ClaudeDo.Installer.Steps;
namespace ClaudeDo.Installer.Tests;
public sealed class RegisterMcpStepTests
{
// Progress<T> posts to the SynchronizationContext (or the ThreadPool) asynchronously,
// so asserting on captured messages right after an await needs a synchronous reporter.
private sealed class SyncProgress<T>(Action<T> report) : IProgress<T>
{
public void Report(T value) => report(value);
}
[Fact]
public void ResolveUrl_uses_the_configured_port()
{
var url = RegisterMcpStep.ResolveUrl(new InstallerWorkerConfig { ExternalMcpPort = 49999 });
Assert.Equal("http://127.0.0.1:49999/mcp", url);
}
[Fact]
public void ResolveUrl_defaults_to_47822_without_a_configured_port()
{
var url = RegisterMcpStep.ResolveUrl(new InstallerWorkerConfig());
Assert.Equal("http://127.0.0.1:47822/mcp", url);
}
[Fact]
public void ResolveUrl_is_null_when_the_external_listener_is_disabled()
{
var url = RegisterMcpStep.ResolveUrl(new InstallerWorkerConfig { ExternalMcpPort = 0 });
Assert.Null(url);
}
[Fact]
public async Task Not_selected_skips_without_loading_the_worker_config()
{
var loaded = false;
var step = new RegisterMcpStep(() => { loaded = true; return new InstallerWorkerConfig(); });
var ctx = new InstallContext { RegisterMcpWithClaude = false };
var result = await step.ExecuteAsync(ctx, new Progress<string>(_ => { }), CancellationToken.None);
Assert.True(result.Success);
Assert.False(loaded);
}
[Fact]
public async Task Disabled_external_port_skips_without_running_the_claude_cli()
{
var step = new RegisterMcpStep(() => new InstallerWorkerConfig { ExternalMcpPort = 0 });
var ctx = new InstallContext { RegisterMcpWithClaude = true };
var messages = new List<string>();
var result = await step.ExecuteAsync(ctx, new SyncProgress<string>(messages.Add), CancellationToken.None);
Assert.True(result.Success);
Assert.Contains(messages, m => m.Contains("external_mcp_port is 0", StringComparison.Ordinal));
}
}