Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion src/Capacitor.Cli/Commands/SetupCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1442,6 +1442,15 @@ internal static VisibilityDecision DecideVisibility(FirstRunAgentsAnswer? browse
/// same as "not found" and draws no alarm on the screen.</summary>
static readonly TimeSpan LoginShellProbeBudget = TimeSpan.FromSeconds(12);

/// <summary>
/// The profile the import lane runs under. <see cref="ImportCommand"/> reads its base URL from the
/// context it is handed, and <c>profiles</c> 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.
/// </summary>
internal static ProfileContext ImportContext(ProfileContext profiles, string serverUrl) =>
new(profiles.Resolution with { ServerUrl = serverUrl }, profiles.Snapshot);

/// <summary>Creates the first-run flow, opens the browser on it, and polls it as itself, returning
/// the Agents decision for Step 4 to apply. <b>Nothing is configured here</b> — what crosses is
/// vendor keys and booleans, and the install runs through the same one place the terminal prompt
Expand Down Expand Up @@ -1490,7 +1499,7 @@ async Task<BrowserFlowAnswers> 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();

Expand Down
2 changes: 1 addition & 1 deletion src/Capacitor.Cli/InteractiveLifetime.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions src/Capacitor.Cli/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
};
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
using System.Runtime.CompilerServices;
using Capacitor.Cli.Commands;

namespace Capacitor.Cli.Tests.Unit.Commands;

/// <summary>
/// <see cref="SetupImportLane"/> imports through <c>ImportCommand</c>, which takes its base URL from
/// the <c>ProfileContext</c> 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 <c>setup</c> 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.
///
/// <para>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
/// <see cref="SetupCommand.ImportContext"/> alone would pass with nothing calling it.</para>
/// </summary>
public class SetupImportLaneConstructionGuardTests {
/// <summary>Walks up from this file to the repo-root marker, so the test runner's working
/// directory is irrelevant.</summary>
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");
}
}
}
30 changes: 30 additions & 0 deletions test/Capacitor.Cli.Tests.Unit/Commands/SetupImportLaneTests.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -413,4 +414,33 @@ public async Task A_clean_run_reports_totals_rather_than_null() {

await Assert.That(totals).IsNotNull();
}

/// <summary>
/// The lane imports through <see cref="ImportCommand"/>, 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 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.
/// </summary>
[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");
}

/// <summary>Only the server moves: the profile the run is configuring still decides everything the
/// import reads off it, visibility included.</summary>
[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);
}
}
39 changes: 39 additions & 0 deletions test/Capacitor.Cli.Tests.Unit/FirstRunExitNoticeGuardTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
using System.Runtime.CompilerServices;
using System.Text.RegularExpressions;

namespace Capacitor.Cli.Tests.Unit;

/// <summary>
/// A leg waiting on the browser owes it a relinquish, and <c>Environment.Exit</c> pays nothing on the
/// way out: no <c>finally</c>, no <c>using</c> disposal, no <c>catch</c>. The signal handlers reach the
/// notice themselves; every other exit reaches it only through <c>AppDomain.ProcessExit</c>, which
/// <c>Environment.Exit</c> does raise.
///
/// <para>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.</para>
/// </summary>
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");
}
}