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
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,15 @@ public void BuildDaemonUnitContent_WiresEnvironmentFile_AndOmitsInlinePath()
Assert.DoesNotContain("Environment=PATH=", unit, StringComparison.Ordinal);
Assert.Contains("ExecStart=/opt/netclaw/netclawd", unit, StringComparison.Ordinal);
Assert.Contains("ExecStop=/opt/netclaw/netclaw daemon stop", unit, StringComparison.Ordinal);

// Producer/consumer contract (netclaw-dev/netclaw#1665): the generated unit's
// TimeoutStopSec= must track DaemonConfig.SystemdTimeoutStopSec, not a stale literal,
// so systemd never SIGKILLs the cgroup out from under a still-legitimately-waiting
// `netclaw daemon stop` (ExecStop=).
Assert.Contains(

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

$"TimeoutStopSec={(int)DaemonConfig.SystemdTimeoutStopSec.TotalSeconds}",
unit,
StringComparison.Ordinal);
}

[Fact]
Expand Down
57 changes: 52 additions & 5 deletions src/Netclaw.Cli/Daemon/DaemonManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -177,10 +177,29 @@ await http.PostAsync(
process.Kill();
}

// Wait up to 10 seconds for graceful exit.
if (!await WaitForExitAsync(process, TimeSpan.FromSeconds(10), cancellationToken))
{
// Timed out — hard cutoff.
// Wait for graceful exit. DaemonConfig.GracefulShutdownBudget matches the daemon's own
// Akka CoordinatedShutdown "before-service-unbind" phase timeout — where session
// draining (SessionDrainHelper.DrainAsync) actually happens — so the CLI does not give
// up on a daemon that is still legitimately draining in-flight sessions (TurnLlmTimeout
// defaults to 3 minutes). See DaemonConfig.GracefulShutdownBudget remarks for the full
// layering this must respect: bounded drain < Akka phase timeout < this budget + the
// grace window below < systemd's TimeoutStopSec= (netclaw-dev/netclaw#1664, #1665).
var exitedWithinBudget = await WaitForExitAsync(process, DaemonConfig.GracefulShutdownBudget, cancellationToken);
var exitedDuringGraceWindow = false;
if (!exitedWithinBudget)
{
// The daemon's own bounded drain (netclaw-dev/netclaw#1664) can time out at almost
// exactly this same budget boundary, and still needs to finish tearing down (actor
// system termination, PID file cleanup) afterward. Poll a short additional grace
// window before escalating to SIGKILL — production evidence (#1665) showed a
// daemon force-killed ~100ms from a clean exit because the CLI escalated the
// instant its budget elapsed, with no headroom at all.
exitedDuringGraceWindow = await WaitForExitAsync(process, DaemonConfig.CliForceKillGraceWindow, cancellationToken);
}

if (!exitedWithinBudget && !exitedDuringGraceWindow)
{
// Both the budget and the grace window elapsed — hard cutoff.
string? killError = null;
if (OperatingSystem.IsLinux() || OperatingSystem.IsMacOS())
{
Expand All @@ -203,7 +222,27 @@ await http.PostAsync(
}

CleanupPidFile();
return new DaemonResult(true, $"Daemon stopped (was PID {pid}).");

if (exitedWithinBudget)
return new DaemonResult(true, $"Daemon stopped (was PID {pid}).");

if (exitedDuringGraceWindow)
{
// Clean exit — no kill needed — but worth surfacing: the daemon used its full
// graceful-shutdown budget, which usually means a session was still mid-LLM-call
// at shutdown.
return new DaemonResult(true,
$"Daemon stopped (was PID {pid}); exited during the " +
$"{DaemonConfig.CliForceKillGraceWindow.TotalSeconds:F0}s grace window after the " +
$"{DaemonConfig.GracefulShutdownBudget.TotalSeconds:F0}s graceful-wait budget elapsed " +
"(no kill needed).");
}

return new DaemonResult(true,
$"Daemon stopped (was PID {pid}), but did not exit gracefully within " +
$"{DaemonConfig.CliForceKillBudget.TotalSeconds:F0}s (budget + grace window) and had to be " +
"force-killed. This usually means a session was still mid-LLM-call at shutdown; if it " +
"recurs, check for stuck sessions before stopping the daemon.");
}

/// <summary>
Expand Down Expand Up @@ -358,6 +397,13 @@ internal void RemoveDaemonEnvironmentFile()
/// The <c>-</c> prefix makes systemd tolerant of a missing env file: a deleted PATH
/// file degrades tool resolution (which <c>SystemdUnitPathDoctorCheck</c> flags)
/// rather than preventing the entire daemon from starting.
///
/// <c>TimeoutStopSec=</c> is <see cref="DaemonConfig.SystemdTimeoutStopSec"/> — comfortably
/// longer than this <see cref="StopAsync"/>'s own graceful-wait-plus-grace budget
/// (<see cref="DaemonConfig.CliForceKillBudget"/>) so systemd never SIGKILLs the whole
/// cgroup out from under <c>ExecStop=</c> (<c>netclaw daemon stop</c>) while that command is
/// still legitimately waiting on the daemon's own shutdown (netclaw-dev/netclaw#1665).
/// Existing installs only pick this up after re-running <c>netclaw daemon install</c>.
/// </summary>
internal static string BuildDaemonUnitContent(string binaryPath, string cliBinaryPath, string environmentFilePath) => $"""
[Unit]
Expand All @@ -368,6 +414,7 @@ internal static string BuildDaemonUnitContent(string binaryPath, string cliBinar
Type=simple
ExecStart={binaryPath}
ExecStop={cliBinaryPath} daemon stop
TimeoutStopSec={(int)DaemonConfig.SystemdTimeoutStopSec.TotalSeconds}
Restart=always
RestartSec=5
Environment=DOTNET_ENVIRONMENT=Production
Expand Down
46 changes: 46 additions & 0 deletions src/Netclaw.Configuration.Tests/DaemonConfigTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -287,4 +287,50 @@ public void Validator_rejects_invalid_trusted_proxy_entry()

Assert.Contains(issues, issue => issue.Message.Contains("not-an-ip", StringComparison.OrdinalIgnoreCase));
}

// ── Shutdown-budget layering (netclaw-dev/netclaw#1664, #1665) ───────────
//
// Four surfaces derive from DaemonConfig.GracefulShutdownBudget and must stay strictly
// ordered: the daemon-stop drain bound, the Akka before-service-unbind phase timeout, the
// CLI's force-kill budget (graceful wait + grace window), and the generated systemd unit's
// TimeoutStopSec=. If any margin is changed inconsistently in the future — e.g. the CLI's
// grace window grows past the systemd teardown margin — this test fails instead of silently
// reintroducing the guaranteed-SIGKILL race these issues were filed against.

[Fact]
public void ShutdownBudgetLayering_bounded_drain_finishes_before_the_akka_phase_timeout()
{
Assert.True(
DaemonConfig.BoundedDrainTimeout < DaemonConfig.GracefulShutdownBudget,
"the daemon-stop drain's own deadline must fire before the Akka phase timeout abandons the task outright");
}

[Fact]
public void ShutdownBudgetLayering_cli_waits_at_least_as_long_as_the_daemon_phase_timeout()
{
Assert.True(
DaemonConfig.GracefulShutdownBudget <= DaemonConfig.CliForceKillBudget,
"the CLI's graceful-wait budget must be at least the daemon's own Akka phase timeout");
}

[Fact]
public void ShutdownBudgetLayering_cli_force_kill_budget_stays_under_systemd_timeout_stop_sec()
{
Assert.True(
DaemonConfig.CliForceKillBudget < DaemonConfig.SystemdTimeoutStopSec,
"systemd's TimeoutStopSec= must exceed the CLI's own budget+grace, or systemd SIGKILLs " +
"the cgroup out from under a `netclaw daemon stop` (ExecStop=) that is still legitimately waiting");
}

[Fact]
public void ShutdownBudgetLayering_matches_the_documented_second_values()
{
// Pins the concrete values referenced in netclaw-dev/netclaw#1665's evidence trail
// (200s phase timeout, 230s TimeoutStopSec) so a change to any constant is a visible,
// deliberate diff rather than a silent drift.
Assert.Equal(TimeSpan.FromSeconds(190), DaemonConfig.BoundedDrainTimeout);
Assert.Equal(TimeSpan.FromSeconds(200), DaemonConfig.GracefulShutdownBudget);
Assert.Equal(TimeSpan.FromSeconds(215), DaemonConfig.CliForceKillBudget);
Assert.Equal(TimeSpan.FromSeconds(230), DaemonConfig.SystemdTimeoutStopSec);
}
}
78 changes: 78 additions & 0 deletions src/Netclaw.Configuration/DaemonConfig.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,84 @@ public sealed record DaemonConfig
/// </summary>
public const int DefaultPort = 5199;

/// <summary>
/// Worst-case time the daemon's graceful shutdown drain is allotted before something gives
/// up and forces termination. Sized to comfortably exceed <c>SessionConfig.TurnLlmTimeout</c>'s
/// default (3 minutes) so a session mid-LLM-call during shutdown can finish draining instead
/// of being interrupted.
///
/// Single source of truth shared by every shutdown-timing surface that must stay in
/// lockstep (netclaw-dev/netclaw#1664, #1665):
/// - Netclaw.Daemon's Akka <c>coordinated-shutdown.phases.before-service-unbind.timeout</c>
/// HOCON override (see <c>DaemonShutdownConfiguration.BuildCoordinatedShutdownHocon</c>),
/// where <c>SessionDrainHelper.DrainAsync</c> actually drains sessions
/// - <see cref="BoundedDrainTimeout"/>, the deadline the daemon-stop CoordinatedShutdown
/// task gives that same drain call — always strictly under this budget, so the drain
/// finishes (timed out or not) before the phase timeout above abandons it outright
/// - Netclaw.Daemon's generic-host <c>HostOptions.ShutdownTimeout</c>
/// - <c>Netclaw.Cli.Daemon.DaemonManager</c>'s graceful-wait before force-kill in
/// <c>netclaw daemon stop</c>, followed by <see cref="CliForceKillGraceWindow"/>
/// - the generated systemd unit's <c>TimeoutStopSec=</c>
/// (<see cref="SystemdTimeoutStopSec"/>; see <c>DaemonManager.BuildDaemonUnitContent</c>)
///
/// #1664: the SIGTERM/daemon-stop drain previously waited unbounded
/// (<c>CancellationToken.None</c>), so a session parked on interactive tool approval — which
/// cannot ack within any budget — hung the call for the full phase timeout, leaking the
/// abandoned drain task. #1665: the CLI's force-kill wait equaled this exact budget with no
/// headroom, so whenever the drain legitimately used the full budget, the CLI guaranteed a
/// SIGKILL race the daemon could not win (evidence: a production daemon force-killed ~100ms
/// from a clean exit).
/// </summary>
public static readonly TimeSpan GracefulShutdownBudget = TimeSpan.FromSeconds(200);

/// <summary>
/// Safety margin subtracted from <see cref="GracefulShutdownBudget"/> to produce
/// <see cref="BoundedDrainTimeout"/>, so the daemon-stop session drain always finishes —
/// timed out or not — strictly before the Akka <c>before-service-unbind</c> phase timeout
/// itself fires and abandons the drain task.
/// </summary>
public static readonly TimeSpan DrainSafetyMargin = TimeSpan.FromSeconds(10);

/// <summary>
/// Additional time <c>netclaw daemon stop</c> polls for process exit after
/// <see cref="GracefulShutdownBudget"/> elapses, before escalating to SIGKILL. Covers the
/// daemon's own post-drain teardown (actor system termination, PID file cleanup) so a
/// daemon that finishes right at the budget boundary is not killed out from under itself
/// (netclaw-dev/netclaw#1665).
/// </summary>
public static readonly TimeSpan CliForceKillGraceWindow = TimeSpan.FromSeconds(15);

/// <summary>
/// Headroom added on top of <see cref="GracefulShutdownBudget"/> for the systemd unit's
/// <c>TimeoutStopSec=</c> (<see cref="SystemdTimeoutStopSec"/>), so systemd never SIGKILLs
/// the whole cgroup out from under <c>ExecStop=</c> (<c>netclaw daemon stop</c>) while that
/// command is still legitimately waiting on the daemon's own bounded shutdown.
/// </summary>
public static readonly TimeSpan SystemdTeardownMargin = TimeSpan.FromSeconds(30);

/// <summary>
/// The deadline the daemon-stop CoordinatedShutdown task gives
/// <c>SessionDrainHelper.DrainAsync</c>. Always strictly less than
/// <see cref="GracefulShutdownBudget"/> so the drain completes — as timed out if
/// necessary — before the Akka phase timeout fires.
/// </summary>
public static TimeSpan BoundedDrainTimeout => GracefulShutdownBudget - DrainSafetyMargin;

/// <summary>
/// Total time <c>netclaw daemon stop</c> allows before escalating to SIGKILL: the
/// graceful wait (matching the daemon's own Akka phase timeout) plus
/// <see cref="CliForceKillGraceWindow"/>.
/// </summary>
public static TimeSpan CliForceKillBudget => GracefulShutdownBudget + CliForceKillGraceWindow;

/// <summary>
/// The systemd unit's <c>TimeoutStopSec=</c> value: comfortably longer than
/// <see cref="CliForceKillBudget"/> so systemd never SIGKILLs the whole cgroup out from
/// under <c>ExecStop=</c> (<c>netclaw daemon stop</c>) while that command is still
/// legitimately waiting on the daemon's own shutdown.
/// </summary>
public static TimeSpan SystemdTimeoutStopSec => GracefulShutdownBudget + SystemdTeardownMargin;

/// <summary>
/// IP address the daemon binds to. Defaults to loopback (<c>127.0.0.1</c>).
/// </summary>
Expand Down
47 changes: 47 additions & 0 deletions src/Netclaw.Daemon.Tests/DaemonShutdownConfigurationTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// -----------------------------------------------------------------------
// <copyright file="DaemonShutdownConfigurationTests.cs" company="Petabridge, LLC">
// Copyright (C) 2026 - 2026 Petabridge, LLC <https://petabridge.com>
// </copyright>
// -----------------------------------------------------------------------
using Akka.Configuration;
using Netclaw.Configuration;
using Xunit;

namespace Netclaw.Daemon.Tests;

/// <summary>
/// Tests for <see cref="DaemonShutdownConfiguration.BuildCoordinatedShutdownHocon"/>, which
/// replaced the inline HOCON literal Program.cs used to prepend a hardcoded 200s
/// before-service-unbind phase timeout (netclaw-dev/netclaw#1664, #1665). Proves the emitted
/// HOCON tracks whatever <see cref="DaemonConfig.GracefulShutdownBudget"/> resolves to instead
/// of drifting back to a literal that could disagree with the daemon-stop drain's own bound.
/// </summary>
public sealed class DaemonShutdownConfigurationTests
{
[Theory]
[InlineData(200, 200)]
[InlineData(90, 90)]
public void BuildCoordinatedShutdownHocon_interpolates_the_given_budget_in_seconds(
int budgetSeconds, int expectedPhaseTimeoutSeconds)
{
var hocon = DaemonShutdownConfiguration.BuildCoordinatedShutdownHocon(TimeSpan.FromSeconds(budgetSeconds));

var config = ConfigurationFactory.ParseString(hocon);

Assert.Equal(TimeSpan.FromSeconds(expectedPhaseTimeoutSeconds),
config.GetTimeSpan("akka.coordinated-shutdown.phases.before-service-unbind.timeout"));
Assert.False(config.GetBoolean("akka.coordinated-shutdown.exit-clr"));
}

[Fact]
public void BuildCoordinatedShutdownHocon_tracks_DaemonConfig_GracefulShutdownBudget()
{
var hocon = DaemonShutdownConfiguration.BuildCoordinatedShutdownHocon(DaemonConfig.GracefulShutdownBudget);

var config = ConfigurationFactory.ParseString(hocon);

Assert.Equal(
DaemonConfig.GracefulShutdownBudget,
config.GetTimeSpan("akka.coordinated-shutdown.phases.before-service-unbind.timeout"));
}
}
36 changes: 36 additions & 0 deletions src/Netclaw.Daemon.Tests/Services/DaemonRestartCoordinatorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,42 @@ public async Task SessionDrainHelper_propagates_caller_cancellation()
await Assert.ThrowsAnyAsync<OperationCanceledException>(() => operation);
}

[Fact]
public async Task SessionDrainHelper_daemon_stop_bound_times_out_instead_of_hanging_when_a_session_never_acks()
{
// Mirrors the daemon-stop CoordinatedShutdown drain task wired in Program.cs
// (netclaw-dev/netclaw#1664): a session whose in-flight turn is parked on interactive
// tool approval never acks PrepareForDaemonRestart. Previously this call passed
// CancellationToken.None for the operation token and hung until Akka's own 200s
// before-service-unbind phase timeout abandoned the task. The bounded CTS below —
// sized from DaemonConfig.BoundedDrainTimeout (GracefulShutdownBudget minus
// DrainSafetyMargin) and driven by TimeProvider exactly as Program.cs constructs it —
// must make the drain complete with a timed-out result well before that.
var time = new FakeTimeProvider();
var activeIds = new[] { "slack/approval-parked" };
var drain = new DrainControl(activeIds, activeIds); // never acknowledged
var sessionManager = _system.ActorOf(Props.Create(() => new StubSessionManagerActor(
activeIds,
drain,
throwOnEnumeration: false)));
using var deadlineCts = new CancellationTokenSource(DaemonConfig.BoundedDrainTimeout, time);

var operation = SessionDrainHelper.DrainAsync(
sessionManager,
"daemon-stop",
NullLogger<DaemonRestartCoordinator>.Instance,
deadlineCts.Token,
CancellationToken.None);
await drain.AllRequestsObserved;
time.Advance(DaemonConfig.BoundedDrainTimeout);

var result = await operation;

Assert.Single(result.AllSessionIds);
Assert.Empty(result.DrainedSessionIds);
Assert.Equal("slack/approval-parked", Assert.Single(result.TimedOutSessionIds).Value);
}

public async ValueTask DisposeAsync()
{
await _system.Terminate();
Expand Down
32 changes: 32 additions & 0 deletions src/Netclaw.Daemon/DaemonShutdownConfiguration.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// -----------------------------------------------------------------------
// <copyright file="DaemonShutdownConfiguration.cs" company="Petabridge, LLC">
// Copyright (C) 2026 - 2026 Petabridge, LLC <https://petabridge.com>
// </copyright>
// -----------------------------------------------------------------------
using Netclaw.Configuration;

namespace Netclaw.Daemon;

/// <summary>
/// Builds the Akka CoordinatedShutdown HOCON override that bounds the daemon's session-drain
/// phase. Extracted into its own testable method (rather than an inline string literal in
/// Program.cs's top-level statements) so a unit test can assert the interpolated timeout
/// tracks <see cref="DaemonConfig.GracefulShutdownBudget"/> instead of drifting back to a
/// hardcoded literal (see <see cref="DaemonConfig.GracefulShutdownBudget"/> remarks for why
/// that drift is the exact class of bug behind netclaw-dev/netclaw#1664 and #1665).
/// </summary>
internal static class DaemonShutdownConfiguration
{
/// <summary>
/// Coordinated-shutdown HOCON: disables the CLR-exit side effect (the daemon's own
/// restart loop owns process lifetime, not CoordinatedShutdown) and sizes the
/// <c>before-service-unbind</c> phase — where session draining
/// (<c>SessionDrainHelper.DrainAsync</c>) actually runs — to <paramref name="gracefulShutdownBudget"/>.
/// </summary>
public static string BuildCoordinatedShutdownHocon(TimeSpan gracefulShutdownBudget) => $$"""
akka.coordinated-shutdown {
exit-clr = off
phases.before-service-unbind.timeout = {{(int)gracefulShutdownBudget.TotalSeconds}}s
}
""";
}
Loading
Loading