From 03ee7f3a6e6e53c48670a350b21866159c9affcd Mon Sep 17 00:00:00 2001 From: George Payne Date: Fri, 4 Sep 2026 12:15:00 +0200 Subject: [PATCH 1/3] Import against the server this run resolved, not the one it started with MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The context handed to the browser import lane is the snapshot taken before Step 1, and setup is exempt from the entry gate that refuses an unconfigured command — so on a first run nothing between the two catches an empty URL. --- src/Capacitor.Cli/Commands/SetupCommand.cs | 11 ++++- .../SetupImportLaneConstructionGuardTests.cs | 44 +++++++++++++++++++ .../Commands/SetupImportLaneTests.cs | 30 +++++++++++++ 3 files changed, 84 insertions(+), 1 deletion(-) create mode 100644 test/Capacitor.Cli.Tests.Unit/Commands/SetupImportLaneConstructionGuardTests.cs diff --git a/src/Capacitor.Cli/Commands/SetupCommand.cs b/src/Capacitor.Cli/Commands/SetupCommand.cs index 5bf6ba6dfd..9596965ea1 100644 --- a/src/Capacitor.Cli/Commands/SetupCommand.cs +++ b/src/Capacitor.Cli/Commands/SetupCommand.cs @@ -1442,6 +1442,15 @@ internal static VisibilityDecision DecideVisibility(FirstRunAgentsAnswer? browse /// same as "not found" and draws no alarm on the screen. static readonly TimeSpan LoginShellProbeBudget = TimeSpan.FromSeconds(12); + /// + /// The profile the import lane runs under. reads its base URL from the + /// context it is handed, and profiles is the snapshot resolved before Step 1 — on a first run + /// that names no server at all, and the one this run chose is not persisted until after this leg + /// returns. + /// + internal static ProfileContext ImportContext(ProfileContext profiles, string serverUrl) => + new(profiles.Resolution with { ServerUrl = serverUrl }, profiles.Snapshot); + /// Creates the first-run flow, opens the browser on it, and polls it as itself, returning /// the Agents decision for Step 4 to apply. Nothing is configured here — what crosses is /// vendor keys and booleans, and the install runs through the same one place the terminal prompt @@ -1490,7 +1499,7 @@ async Task RunBrowserFlowStepAsync(string serverUrl, string config, HarnessRegistry.FromEnvironment(home), Environment.MachineName, await LoginShellFindsCliAsync()); - importing = new SetupImportLane(config, profiles, home, _paths); + importing = new SetupImportLane(config, ImportContext(profiles, serverUrl), home, _paths); using var progress = new SpectreFirstRunFlowProgress(); diff --git a/test/Capacitor.Cli.Tests.Unit/Commands/SetupImportLaneConstructionGuardTests.cs b/test/Capacitor.Cli.Tests.Unit/Commands/SetupImportLaneConstructionGuardTests.cs new file mode 100644 index 0000000000..4421317e27 --- /dev/null +++ b/test/Capacitor.Cli.Tests.Unit/Commands/SetupImportLaneConstructionGuardTests.cs @@ -0,0 +1,44 @@ +using System.Runtime.CompilerServices; +using Capacitor.Cli.Commands; + +namespace Capacitor.Cli.Tests.Unit.Commands; + +/// +/// imports through ImportCommand, which takes its base URL from +/// the ProfileContext it is handed and from nowhere else. The browser leg's own context is the +/// snapshot resolved before Step 1, so on a first run it names no server — and setup is exempt +/// from the entry gate that refuses an unconfigured command, so an unusable URL is caught by nothing +/// until it reaches the client factory. +/// +/// A guard rather than a unit test because the defect is in WHICH context the call site passes, +/// and the call site sits inside an authenticated leg that a unit test cannot reach. Pinning +/// alone would pass with nothing calling it. +/// +public class SetupImportLaneConstructionGuardTests { + /// Walks up from this file to the repo-root marker, so the test runner's working + /// directory is irrelevant. + static string RepoRoot([CallerFilePath] string here = "") { + var dir = Path.GetDirectoryName(here); + while (dir is not null && !File.Exists(Path.Combine(dir, "Capacitor.slnx"))) + dir = Path.GetDirectoryName(dir); + + return dir ?? throw new InvalidOperationException($"Could not locate repo root walking up from {here}"); + } + + [Test] + public async Task Every_construction_of_the_lane_names_the_server_this_run_resolved() { + var sites = Directory + .EnumerateFiles(Path.Combine(RepoRoot(), "src"), "*.cs", SearchOption.AllDirectories) + .SelectMany(f => File.ReadAllLines(f).Select((line, i) => (File: Path.GetFileName(f), No: i + 1, line))) + .Where(l => l.line.Contains("new SetupImportLane(", StringComparison.Ordinal)) + .ToList(); + + await Assert.That(sites).IsNotEmpty() + .Because("a rename that empties this scan would make the guard pass for the wrong reason"); + + foreach (var site in sites) { + await Assert.That(site.line).Contains("ImportContext(") + .Because($"{site.File}:{site.No} hands the lane a context that need not name a server"); + } + } +} diff --git a/test/Capacitor.Cli.Tests.Unit/Commands/SetupImportLaneTests.cs b/test/Capacitor.Cli.Tests.Unit/Commands/SetupImportLaneTests.cs index 5152bf4db6..849a6da11d 100644 --- a/test/Capacitor.Cli.Tests.Unit/Commands/SetupImportLaneTests.cs +++ b/test/Capacitor.Cli.Tests.Unit/Commands/SetupImportLaneTests.cs @@ -1,4 +1,5 @@ using Capacitor.Cli.Commands; +using Capacitor.Cli.Core.Config; using Capacitor.Cli.Core.FirstRun; using Capacitor.Cli.Core.Harness; using Capacitor.Cli.Harness.Claude; @@ -413,4 +414,33 @@ public async Task A_clean_run_reports_totals_rather_than_null() { await Assert.That(totals).IsNotNull(); } + + /// + /// The lane imports through , which takes its base URL from the context + /// it is handed — so the one this run resolved has to reach it. The leg's own context is the + /// snapshot from before Step 1, and on a first run that names no server at all: `setup` is exempt + /// from the entry gate that refuses an unconfigured command, so nothing downstream would catch it. + /// + [Test] + public async Task The_import_context_names_the_server_this_run_resolved() { + var beforeSetup = new ProfileContext(new(null, "default", null, null), new ProfileConfig()); + + var forImport = SetupCommand.ImportContext(beforeSetup, "https://tenant.kcap.ai"); + + await Assert.That(forImport.Resolution.ServerUrl).IsEqualTo("https://tenant.kcap.ai"); + } + + /// Only the server moves: the profile the run is configuring still decides everything the + /// import reads off it, visibility included. + [Test] + public async Task The_import_context_keeps_the_rest_of_the_resolution() { + var profile = new Profile { ServerUrl = "https://stale.kcap.ai", DefaultVisibility = "org_public" }; + var beforeSetup = new ProfileContext(new("https://stale.kcap.ai", "work", profile, null), new ProfileConfig()); + + var forImport = SetupCommand.ImportContext(beforeSetup, "https://tenant.kcap.ai"); + + await Assert.That(forImport.Resolution.ProfileName).IsEqualTo("work"); + await Assert.That(forImport.Resolution.Profile).IsSameReferenceAs(profile); + await Assert.That(forImport.Snapshot).IsSameReferenceAs(beforeSetup.Snapshot); + } } From 6cbbd755b2e33ca3d169d73c0997c717bca79fe6 Mon Sep 17 00:00:00 2001 From: George Payne Date: Fri, 4 Sep 2026 12:15:05 +0200 Subject: [PATCH 2/3] Send the pending first-run relinquish from ProcessExit Environment.Exit runs no finally and disposes nothing, so a leg that ends that way never sends the notice it armed and the browser waits out the flow's whole lifetime. ProcessExit is the one hook Environment.Exit raises. --- src/Capacitor.Cli/InteractiveLifetime.cs | 2 +- src/Capacitor.Cli/Program.cs | 7 ++++ .../FirstRunExitNoticeGuardTests.cs | 39 +++++++++++++++++++ 3 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 test/Capacitor.Cli.Tests.Unit/FirstRunExitNoticeGuardTests.cs diff --git a/src/Capacitor.Cli/InteractiveLifetime.cs b/src/Capacitor.Cli/InteractiveLifetime.cs index afdc9cca0a..77b615f8d2 100644 --- a/src/Capacitor.Cli/InteractiveLifetime.cs +++ b/src/Capacitor.Cli/InteractiveLifetime.cs @@ -38,7 +38,7 @@ static class InteractiveLifetime { // What an interrupt may spend telling a server the machine has gone. A whole second of an exit // the user has already asked for is as much as this is worth: one small POST to a host we are // mid-conversation with either lands well inside it or is not going to. - static readonly TimeSpan ExitNoticeBudget = TimeSpan.FromSeconds(1); + internal static readonly TimeSpan ExitNoticeBudget = TimeSpan.FromSeconds(1); // PosixSignalRegistration.Create returns an IDisposable that owns the // underlying handler slot; if it isn't rooted for the process lifetime the diff --git a/src/Capacitor.Cli/Program.cs b/src/Capacitor.Cli/Program.cs index e2cd2559a5..cc61740c5f 100644 --- a/src/Capacitor.Cli/Program.cs +++ b/src/Capacitor.Cli/Program.cs @@ -7,6 +7,7 @@ using Capacitor.Cli.Core; using Capacitor.Cli.Core.Auth; using Capacitor.Cli.Core.Config; +using Capacitor.Cli.Core.FirstRun; using Capacitor.Cli.Core.Harness; using Capacitor.Cli.Core.Telemetry; using Capacitor.Cli.Harness.Claude; @@ -149,6 +150,12 @@ CliTelemetry.Initialize(command, baseUrl, loggedIn, config, telemetrySuppressed); AppDomain.CurrentDomain.ProcessExit += (_, _) => { + // Environment.Exit runs no `finally` and no `using` disposal, so a leg that ends that way has no + // other chance to tell a waiting browser it has stopped. Ahead of the telemetry flush because both + // spend the runtime's one exit budget and only this one is holding a page open; the notice claims + // once, so a leg that already sent stays silent and a process with none armed is a no-op. + FirstRunInterruptRelinquish.RunBeforeExit(InteractiveLifetime.ExitNoticeBudget); + CliTelemetry.RecordCommand(command, args, Environment.ExitCode, CommandTiming.ElapsedMs(commandStart)); CliTelemetry.FlushAndClose().GetAwaiter().GetResult(); }; diff --git a/test/Capacitor.Cli.Tests.Unit/FirstRunExitNoticeGuardTests.cs b/test/Capacitor.Cli.Tests.Unit/FirstRunExitNoticeGuardTests.cs new file mode 100644 index 0000000000..079a4335c5 --- /dev/null +++ b/test/Capacitor.Cli.Tests.Unit/FirstRunExitNoticeGuardTests.cs @@ -0,0 +1,39 @@ +using System.Runtime.CompilerServices; +using System.Text.RegularExpressions; + +namespace Capacitor.Cli.Tests.Unit; + +/// +/// A leg waiting on the browser owes it a relinquish, and Environment.Exit pays nothing on the +/// way out: no finally, no using disposal, no catch. The signal handlers reach the +/// notice themselves; every other exit reaches it only through AppDomain.ProcessExit, which +/// Environment.Exit does raise. +/// +/// Pinned as a guard because the failure is silent and remote — the process ends cleanly, the +/// terminal looks finished, and what breaks is a page on another machine that never stops waiting. +/// Nothing in the CLI's own output would show it. +/// +public class FirstRunExitNoticeGuardTests { + static string RepoRoot([CallerFilePath] string here = "") { + var dir = Path.GetDirectoryName(here); + while (dir is not null && !File.Exists(Path.Combine(dir, "Capacitor.slnx"))) + dir = Path.GetDirectoryName(dir); + + return dir ?? throw new InvalidOperationException($"Could not locate repo root walking up from {here}"); + } + + [Test] + public async Task The_process_exit_handler_sends_the_pending_relinquish() { + var program = await File.ReadAllTextAsync( + Path.Combine(RepoRoot(), "src", "Capacitor.Cli", "Program.cs")); + + var handler = Regex.Match( + program, @"AppDomain\.CurrentDomain\.ProcessExit\s*\+=.*?\n\};", RegexOptions.Singleline); + + await Assert.That(handler.Success).IsTrue() + .Because("no ProcessExit handler means no exit path reaches the notice at all"); + + await Assert.That(handler.Value).Contains("FirstRunInterruptRelinquish.RunBeforeExit") + .Because("an Environment.Exit anywhere in the CLI otherwise leaves the browser waiting"); + } +} From 1c21ce757bbe25745a44bbca1509d6e538b20310 Mon Sep 17 00:00:00 2001 From: George Payne Date: Fri, 4 Sep 2026 12:19:59 +0200 Subject: [PATCH 3/3] Name the point the server is resolved, not the step that renders it A step number drifts if the flow is reordered; what the context is missing is a server, and the point it is missing one at is process start. --- src/Capacitor.Cli/Commands/SetupCommand.cs | 6 +++--- .../Commands/SetupImportLaneConstructionGuardTests.cs | 8 ++++---- .../Commands/SetupImportLaneTests.cs | 6 +++--- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/Capacitor.Cli/Commands/SetupCommand.cs b/src/Capacitor.Cli/Commands/SetupCommand.cs index 9596965ea1..6a43c22492 100644 --- a/src/Capacitor.Cli/Commands/SetupCommand.cs +++ b/src/Capacitor.Cli/Commands/SetupCommand.cs @@ -1444,9 +1444,9 @@ internal static VisibilityDecision DecideVisibility(FirstRunAgentsAnswer? browse /// /// The profile the import lane runs under. reads its base URL from the - /// context it is handed, and profiles is the snapshot resolved before Step 1 — on a first run - /// that names no server at all, and the one this run chose is not persisted until after this leg - /// returns. + /// context it is handed, and profiles is resolved at process start, before setup has asked + /// for a server — on a first run it names none at all, and the one this run chose is not persisted + /// until after this leg returns. /// internal static ProfileContext ImportContext(ProfileContext profiles, string serverUrl) => new(profiles.Resolution with { ServerUrl = serverUrl }, profiles.Snapshot); diff --git a/test/Capacitor.Cli.Tests.Unit/Commands/SetupImportLaneConstructionGuardTests.cs b/test/Capacitor.Cli.Tests.Unit/Commands/SetupImportLaneConstructionGuardTests.cs index 4421317e27..532c988892 100644 --- a/test/Capacitor.Cli.Tests.Unit/Commands/SetupImportLaneConstructionGuardTests.cs +++ b/test/Capacitor.Cli.Tests.Unit/Commands/SetupImportLaneConstructionGuardTests.cs @@ -5,10 +5,10 @@ namespace Capacitor.Cli.Tests.Unit.Commands; /// /// imports through ImportCommand, which takes its base URL from -/// the ProfileContext it is handed and from nowhere else. The browser leg's own context is the -/// snapshot resolved before Step 1, so on a first run it names no server — and setup is exempt -/// from the entry gate that refuses an unconfigured command, so an unusable URL is caught by nothing -/// until it reaches the client factory. +/// the ProfileContext it is handed and from nowhere else. The browser leg's own context is +/// resolved at process start, before setup has asked for a server, so on a first run it names none — +/// and setup is exempt from the entry gate that refuses an unconfigured command, so an unusable +/// URL is caught by nothing until it reaches the client factory. /// /// A guard rather than a unit test because the defect is in WHICH context the call site passes, /// and the call site sits inside an authenticated leg that a unit test cannot reach. Pinning diff --git a/test/Capacitor.Cli.Tests.Unit/Commands/SetupImportLaneTests.cs b/test/Capacitor.Cli.Tests.Unit/Commands/SetupImportLaneTests.cs index 849a6da11d..848108f7cb 100644 --- a/test/Capacitor.Cli.Tests.Unit/Commands/SetupImportLaneTests.cs +++ b/test/Capacitor.Cli.Tests.Unit/Commands/SetupImportLaneTests.cs @@ -417,9 +417,9 @@ public async Task A_clean_run_reports_totals_rather_than_null() { /// /// The lane imports through , which takes its base URL from the context - /// it is handed — so the one this run resolved has to reach it. The leg's own context is the - /// snapshot from before Step 1, and on a first run that names no server at all: `setup` is exempt - /// from the entry gate that refuses an unconfigured command, so nothing downstream would catch it. + /// it is handed — so the one this run resolved has to reach it. The leg's own context is resolved at + /// process start, before setup has asked for a server, and on a first run it names none: `setup` is + /// exempt from the entry gate that refuses an unconfigured command, so nothing downstream catches it. /// [Test] public async Task The_import_context_names_the_server_this_run_resolved() {