diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Program/InstallModeMergeTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Program/InstallModeMergeTests.cs index 72f35c11..a2e490b0 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Program/InstallModeMergeTests.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Program/InstallModeMergeTests.cs @@ -132,10 +132,12 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Program } [Fact] - public void First_install_without_wait_arg_uses_agent_default_of_600() + public void First_install_without_wait_arg_persists_null_so_runtime_default_applies() { // Generic bootstrap path: PS1 calls `--install` without --tenant-id-wait, - // and there is no persisted config yet. Agent owns the default (600 s). + // and there is no persisted config yet. "Not configured" is persisted as null + // so BuildAgentConfiguration applies the agent-side default (600 s) — freezing + // a number into the file would decouple deployed devices from default changes. var merged = AutopilotMonitor.Agent.V2.Program.MergeBootstrapConfig( existing: null, bootstrapTokenArg: null!, bootstrapTokenGiven: false, @@ -144,20 +146,42 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Program Assert.Null(merged.BootstrapToken); Assert.Null(merged.TenantId); - Assert.Equal(600, merged.TenantIdWaitSeconds); + Assert.Null(merged.TenantIdWaitSeconds); } [Fact] - public void Existing_config_without_wait_field_treated_as_zero() + public void Legacy_config_without_wait_field_stays_null_on_plain_reinstall() { - // BackCompat: pre-fix bootstrap-config.json has no TenantIdWaitSeconds field; - // Newtonsoft deserialises that to 0. New install with --tenant-id-wait 600 - // overrides cleanly; no extra migration logic required. + // BackCompat: pre-feature bootstrap-config.json has no TenantIdWaitSeconds + // property → nullable deserialisation yields null. A reinstall that does not + // pass --tenant-id-wait must NOT turn that into 0 (which would silently + // disable the wait); null carries through and the runtime default applies. var existing = new BootstrapConfigFile { BootstrapToken = "tok-legacy", TenantId = "tenant-legacy", - TenantIdWaitSeconds = 0, + TenantIdWaitSeconds = null, + }; + + var merged = AutopilotMonitor.Agent.V2.Program.MergeBootstrapConfig( + existing, + bootstrapTokenArg: null!, bootstrapTokenGiven: false, + tenantIdArg: null!, tenantIdGiven: false, + tenantIdWaitSeconds: 0, tenantIdWaitGiven: false); + + Assert.Equal("tok-legacy", merged.BootstrapToken); + Assert.Equal("tenant-legacy", merged.TenantId); + Assert.Null(merged.TenantIdWaitSeconds); + } + + [Fact] + public void Legacy_config_without_wait_field_takes_explicit_wait_arg() + { + var existing = new BootstrapConfigFile + { + BootstrapToken = "tok-legacy", + TenantId = "tenant-legacy", + TenantIdWaitSeconds = null, }; var merged = AutopilotMonitor.Agent.V2.Program.MergeBootstrapConfig( @@ -170,5 +194,26 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Program Assert.Equal("tenant-legacy", merged.TenantId); Assert.Equal(600, merged.TenantIdWaitSeconds); } + + [Fact] + public void Explicit_zero_opt_out_survives_redeploy_without_wait_arg() + { + // An admin's deliberate `--tenant-id-wait 0` must not be resurrected to a + // wait by a later redeploy that omits the flag. + var existing = new BootstrapConfigFile + { + BootstrapToken = "tok-abc", + TenantId = "tenant-xyz", + TenantIdWaitSeconds = 0, + }; + + var merged = AutopilotMonitor.Agent.V2.Program.MergeBootstrapConfig( + existing, + bootstrapTokenArg: null!, bootstrapTokenGiven: false, + tenantIdArg: null!, tenantIdGiven: false, + tenantIdWaitSeconds: 0, tenantIdWaitGiven: false); + + Assert.Equal(0, merged.TenantIdWaitSeconds); + } } } diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Program/TenantIdWaitResolutionTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Program/TenantIdWaitResolutionTests.cs new file mode 100644 index 00000000..68fb4739 --- /dev/null +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Program/TenantIdWaitResolutionTests.cs @@ -0,0 +1,90 @@ +using AutopilotMonitor.Agent.V2; +using AutopilotMonitor.Agent.V2.Core.Configuration; +using Xunit; + +namespace AutopilotMonitor.Agent.V2.Core.Tests.Program +{ + /// + /// Pins the TenantIdWaitSeconds resolution order in + /// : + /// CLI arg > persisted bootstrap-config.json value > agent default (600 s). + /// The regression this guards: a legacy bootstrap-config.json written before the + /// TenantIdWaitSeconds property existed deserialises to null — that must fall + /// through to the 600 s default, NOT be treated as an explicit 0 opt-out + /// (which silently disabled the wait and broke Hybrid-AAD-join devices whose + /// registry TenantId lands minutes after first boot). + /// + public sealed class TenantIdWaitResolutionTests + { + private static AgentConfiguration Build(BootstrapConfigFile bootstrapConfig, params string[] args) => + AutopilotMonitor.Agent.V2.Program.BuildAgentConfiguration( + args: args, + tenantId: "tenant-t", + sessionId: "session-s", + bootstrapConfig: bootstrapConfig, + awaitConfig: null); + + [Fact] + public void No_cli_and_no_bootstrap_config_uses_default_600() + { + var cfg = Build(bootstrapConfig: null); + + Assert.Equal(600, cfg.TenantIdWaitSeconds); + } + + [Fact] + public void Legacy_bootstrap_config_with_null_wait_uses_default_600() + { + var legacy = new BootstrapConfigFile + { + BootstrapToken = "tok-legacy", + TenantId = "tenant-legacy", + TenantIdWaitSeconds = null, + }; + + var cfg = Build(legacy); + + Assert.Equal(600, cfg.TenantIdWaitSeconds); + } + + [Fact] + public void Explicit_zero_in_bootstrap_config_disables_wait() + { + var optedOut = new BootstrapConfigFile { TenantIdWaitSeconds = 0 }; + + var cfg = Build(optedOut); + + Assert.Equal(0, cfg.TenantIdWaitSeconds); + } + + [Fact] + public void Nonzero_bootstrap_config_value_wins_over_default() + { + var configured = new BootstrapConfigFile { TenantIdWaitSeconds = 1800 }; + + var cfg = Build(configured); + + Assert.Equal(1800, cfg.TenantIdWaitSeconds); + } + + [Fact] + public void Cli_arg_wins_over_bootstrap_config_value() + { + var configured = new BootstrapConfigFile { TenantIdWaitSeconds = 1800 }; + + var cfg = Build(configured, "--tenant-id-wait", "300"); + + Assert.Equal(300, cfg.TenantIdWaitSeconds); + } + + [Fact] + public void Cli_zero_wins_over_bootstrap_config_value() + { + var configured = new BootstrapConfigFile { TenantIdWaitSeconds = 1800 }; + + var cfg = Build(configured, "--tenant-id-wait", "0"); + + Assert.Equal(0, cfg.TenantIdWaitSeconds); + } + } +} diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Configuration/AgentConfiguration.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Configuration/AgentConfiguration.cs index 6c93d107..5b65ccb7 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Configuration/AgentConfiguration.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Configuration/AgentConfiguration.cs @@ -272,9 +272,10 @@ namespace AutopilotMonitor.Agent.V2.Core.Configuration /// CloudDomainJoin keys and re-probes on every change until either resolution /// or this timeout. 0 = no wait, fast-fail on miss (legacy behaviour). /// - /// Set via the install-time CLI flag --tenant-id-wait <sec>, persisted - /// to bootstrap-config.json. The PS1 bootstrap script owns the production - /// default; the agent itself never assumes one. + /// Set via the CLI flag --tenant-id-wait <sec> or a persisted + /// bootstrap-config.json value; when neither supplies a value the agent + /// applies its own default of 600 s in BuildAgentConfiguration. The 0 default + /// here is only the POCO baseline, not the effective runtime default. /// /// public int TenantIdWaitSeconds { get; set; } = 0; diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Configuration/BootstrapConfigFile.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Configuration/BootstrapConfigFile.cs index 5dd95c1c..cf4d6563 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Configuration/BootstrapConfigFile.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Configuration/BootstrapConfigFile.cs @@ -16,8 +16,11 @@ namespace AutopilotMonitor.Agent.V2.Core.Configuration /// /// TenantId-wait timeout in seconds, persisted from the --tenant-id-wait /// install-time arg. Read by AgentBootstrap when the registry probe finds no - /// TenantId on first try. 0 / missing = no wait, legacy fast-fail. + /// TenantId on first try. 0 = explicit opt-out (legacy fast-fail). + /// Null / missing (files written before this property existed) = not configured — + /// the agent-side default (600 s) applies. Nullable so a legacy file without the + /// property cannot masquerade as an explicit 0 and silently disable the wait. /// - public int TenantIdWaitSeconds { get; set; } + public int? TenantIdWaitSeconds { get; set; } } } diff --git a/src/Agent/AutopilotMonitor.Agent.V2/Program.InstallMode.cs b/src/Agent/AutopilotMonitor.Agent.V2/Program.InstallMode.cs index 81045a00..46736fe0 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2/Program.InstallMode.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2/Program.InstallMode.cs @@ -135,7 +135,7 @@ namespace AutopilotMonitor.Agent.V2 $"{(bootstrapTokenGiven ? "*" : "")}, " + $"tenantId={(string.IsNullOrEmpty(merged.TenantId) ? "no" : "yes")}" + $"{(tenantIdGiven ? "*" : "")}, " + - $"tenantIdWait={merged.TenantIdWaitSeconds}s" + + $"tenantIdWait={(merged.TenantIdWaitSeconds is int w ? $"{w}s" : "default")}" + $"{(tenantIdWaitGiven ? "*" : "")})." + " Star = explicitly set on this --install invocation, others were preserved from prior config."); } @@ -361,9 +361,12 @@ namespace AutopilotMonitor.Agent.V2 { BootstrapToken = bootstrapTokenGiven ? bootstrapTokenArg : existing?.BootstrapToken, TenantId = tenantIdGiven ? tenantIdArg : existing?.TenantId, + // Not given → carry the prior value through, INCLUDING null. Persisting a + // number here would freeze the agent-side default into the file; persisting + // null keeps "not configured" distinguishable from an explicit opt-out (0). TenantIdWaitSeconds = tenantIdWaitGiven ? tenantIdWaitSeconds - : (existing?.TenantIdWaitSeconds ?? 600), + : existing?.TenantIdWaitSeconds, }; } diff --git a/src/Agent/AutopilotMonitor.Agent.V2/Program.cs b/src/Agent/AutopilotMonitor.Agent.V2/Program.cs index af8c5bb8..d4127782 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2/Program.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2/Program.cs @@ -339,13 +339,15 @@ namespace AutopilotMonitor.Agent.V2 // over the agent-side default (600 s). Hybrid-AAD-joined devices typically // need ~5 min for the AAD device cert to land in the registry; 600 s leaves // headroom. Pass `--tenant-id-wait 0` to opt out (legacy fast-fail). + // A bootstrap-config.json whose TenantIdWaitSeconds is null (file predates the + // property) is "not configured", NOT an opt-out — the default must still apply. var tenantIdWaitRaw = GetArgValue(args, "--tenant-id-wait"); const int tenantIdWaitDefaultSeconds = 600; int tenantIdWaitSeconds = tenantIdWaitDefaultSeconds; if (!string.IsNullOrEmpty(tenantIdWaitRaw) && int.TryParse(tenantIdWaitRaw, out var parsedTenantIdWait)) tenantIdWaitSeconds = parsedTenantIdWait; - else if (bootstrapConfig != null) - tenantIdWaitSeconds = bootstrapConfig.TenantIdWaitSeconds; + else if (bootstrapConfig?.TenantIdWaitSeconds != null) + tenantIdWaitSeconds = bootstrapConfig.TenantIdWaitSeconds.Value; var useBootstrapTokenAuth = !string.IsNullOrEmpty(bootstrapToken);