test(v2): extract DeathRattlePrelude so PR3 gate is directly testable

Codex review on PR3: the AgentRuntimeHost integration (gate + path build +
pre-Start read) is the fragile part of the plan and was only tested
indirectly through the helper + emitter paths. A regress like
"TryReadRaw moved to after Start", "WhiteGlove resume not skipped" or
"clean exit fires" would have slipped through.

Pull the gate / path / read sequence into a dedicated DeathRattlePrelude
in V2.Core so it can be exercised in isolation:

- DeathRattlePrelude.TryCapture(stateDirectory, previousExitType,
  isWhiteGloveResume, logger) returns the prior snapshot or null;
  IsUncleanExit exposed for callers that want to log the decision.
- AgentRuntimeHost reduced to a single TryCapture call; private
  IsUncleanExit + System.IO using removed.
- DeathRattlePreludeTests (24 cases) cover:
    - Clean-exit table (clean / first_run / unknown / null) — all skip,
      even with snapshot present.
    - Unclean-exit table (reboot_kill / hard_kill / exception_crash,
      mixed casing) — all read.
    - WhiteGlove Part-2 resume gate wins over unclean exit.
    - Snapshot missing / corrupt → null, never throws.
    - Path-layout pin: prelude reads the same hardcoded "snapshot.json"
      filename the orchestrator writes to.
    - Non-mutation contract: byte-level equality before/after, file
      still loadable via SnapshotPersistence.Load(), no .quarantine
      directory created (orchestrator owns the quarantine decision).
    - Argument validation (null logger / null+empty stateDirectory).

Test stand: V2.Core 983 (+24).
This commit is contained in:
Oliver Kieselbach
2026-05-03 21:40:10 +02:00
parent 6e240fba53
commit 118b4c0a41
4 changed files with 452 additions and 52 deletions
@@ -0,0 +1,331 @@
#nullable enable
using System;
using System.IO;
using AutopilotMonitor.Agent.V2.Core.Logging;
using AutopilotMonitor.Agent.V2.Core.Persistence;
using AutopilotMonitor.Agent.V2.Core.Tests.Harness;
using AutopilotMonitor.DecisionCore.Signals;
using AutopilotMonitor.DecisionCore.State;
using AutopilotMonitor.Shared.Models;
using Xunit;
namespace AutopilotMonitor.Agent.V2.Core.Tests.Persistence
{
/// <summary>
/// Tests for <see cref="DeathRattlePrelude.TryCapture"/> — the extracted gate +
/// path-build + snapshot-read that lives in front of <c>orchestrator.Start()</c> in
/// AgentRuntimeHost. These tests directly cover the three regress classes that the
/// host wiring is fragile against:
/// <list type="bullet">
/// <item>TryReadRaw moved to AFTER Start — caught by the no-mutation contract test
/// (file must be readable bit-identically by a fresh SnapshotPersistence after the
/// prelude returns) and by the path-layout test (prelude reads the same hardcoded
/// "snapshot.json" the orchestrator writes to).</item>
/// <item>WhiteGlove Part-2 resume not skipped — caught by the WG-skip-table test.</item>
/// <item><c>clean</c> / <c>first_run</c> exits firing — caught by the clean-exit-table test.</item>
/// </list>
/// </summary>
public sealed class DeathRattlePreludeTests
{
private static AgentLogger NewLogger(TempDirectory tmp)
=> new AgentLogger(Path.Combine(tmp.Path, "logs"), AgentLogLevel.Debug);
private static DecisionState BuildState(SessionStage stage = SessionStage.AwaitingHello, int stepIndex = 7)
{
var b = DecisionState.CreateInitial("S1", "T1").ToBuilder()
.WithStage(stage)
.WithStepIndex(stepIndex)
.WithLastAppliedSignalOrdinal(stepIndex);
b.DesktopArrivedUtc = new SignalFact<DateTime>(
new DateTime(2026, 5, 1, 13, 45, 37, DateTimeKind.Utc),
sourceSignalOrdinal: 28);
return b.Build();
}
private static string SaveSnapshot(string dir, DecisionState? state = null)
{
var path = Path.Combine(dir, DeathRattlePrelude.SnapshotFileName);
new SnapshotPersistence(path).Save(state ?? BuildState());
return path;
}
// ============================================================================
// Clean-exit table — none of the planned exit classifications triggers
// ============================================================================
[Theory]
[InlineData("clean")]
[InlineData("first_run")]
[InlineData("CLEAN")]
[InlineData("First_Run")]
public void Returns_null_for_planned_exit_types_even_when_snapshot_present(string exitType)
{
using var tmp = new TempDirectory();
SaveSnapshot(tmp.Path);
var result = DeathRattlePrelude.TryCapture(
stateDirectory: tmp.Path,
previousExitType: exitType,
isWhiteGloveResume: false,
logger: NewLogger(tmp));
Assert.Null(result);
}
[Fact]
public void Returns_null_for_null_previousExitType_with_snapshot_present()
{
using var tmp = new TempDirectory();
SaveSnapshot(tmp.Path);
var result = DeathRattlePrelude.TryCapture(
stateDirectory: tmp.Path,
previousExitType: null,
isWhiteGloveResume: false,
logger: NewLogger(tmp));
Assert.Null(result);
}
[Fact]
public void Returns_null_for_unknown_exit_type_with_snapshot_present()
{
// Defensive: if Program.DetectPreviousExit ever produces an unrecognised
// value, the prelude must default to "no death-rattle" rather than
// misclassify it as unclean. Telemetry-noise-by-default is a worse failure
// mode than missed-attestation-on-edge-case.
using var tmp = new TempDirectory();
SaveSnapshot(tmp.Path);
var result = DeathRattlePrelude.TryCapture(
stateDirectory: tmp.Path,
previousExitType: "totally_made_up_type",
isWhiteGloveResume: false,
logger: NewLogger(tmp));
Assert.Null(result);
}
// ============================================================================
// Unclean-exit table — every classification on the allowlist triggers
// ============================================================================
[Theory]
[InlineData("reboot_kill")]
[InlineData("hard_kill")]
[InlineData("exception_crash")]
[InlineData("REBOOT_KILL")] // case-insensitive
[InlineData("Hard_Kill")]
[InlineData("Exception_Crash")]
public void Returns_state_for_unclean_exit_types_when_snapshot_present(string exitType)
{
using var tmp = new TempDirectory();
SaveSnapshot(tmp.Path);
var result = DeathRattlePrelude.TryCapture(
stateDirectory: tmp.Path,
previousExitType: exitType,
isWhiteGloveResume: false,
logger: NewLogger(tmp));
Assert.NotNull(result);
Assert.Equal(SessionStage.AwaitingHello, result!.Stage);
Assert.Equal(7, result.StepIndex);
}
[Fact]
public void IsUncleanExit_pins_the_allowlist_explicitly()
{
// Public gate is also exposed for callers that want to log the decision
// independently. Kept covered so a future "let me also include X" change
// forces the test to be updated alongside the policy.
Assert.True(DeathRattlePrelude.IsUncleanExit("reboot_kill"));
Assert.True(DeathRattlePrelude.IsUncleanExit("hard_kill"));
Assert.True(DeathRattlePrelude.IsUncleanExit("exception_crash"));
Assert.False(DeathRattlePrelude.IsUncleanExit("clean"));
Assert.False(DeathRattlePrelude.IsUncleanExit("first_run"));
Assert.False(DeathRattlePrelude.IsUncleanExit(null));
Assert.False(DeathRattlePrelude.IsUncleanExit(""));
Assert.False(DeathRattlePrelude.IsUncleanExit("totally_made_up"));
}
// ============================================================================
// WhiteGlove-Resume gate — wins even on unclean exit
// ============================================================================
[Theory]
[InlineData("reboot_kill")]
[InlineData("hard_kill")]
[InlineData("exception_crash")]
public void Returns_null_when_isWhiteGloveResume_even_on_unclean_exit(string exitType)
{
// Plan §B: WhiteGlove Part-2 resume's "death" is the planned Part-1 sealing
// exit, which kicks the box through Sysprep + reboot. That's not an unclean
// death — emitting a death-rattle there would be telemetry noise.
using var tmp = new TempDirectory();
SaveSnapshot(tmp.Path);
var result = DeathRattlePrelude.TryCapture(
stateDirectory: tmp.Path,
previousExitType: exitType,
isWhiteGloveResume: true,
logger: NewLogger(tmp));
Assert.Null(result);
}
// ============================================================================
// Snapshot-availability cases — missing / corrupt → null, never throws
// ============================================================================
[Fact]
public void Returns_null_when_snapshot_file_missing()
{
using var tmp = new TempDirectory();
// No SaveSnapshot call.
var result = DeathRattlePrelude.TryCapture(
stateDirectory: tmp.Path,
previousExitType: "reboot_kill",
isWhiteGloveResume: false,
logger: NewLogger(tmp));
Assert.Null(result);
}
[Fact]
public void Returns_null_when_snapshot_corrupt_and_does_not_throw()
{
using var tmp = new TempDirectory();
File.WriteAllText(
Path.Combine(tmp.Path, DeathRattlePrelude.SnapshotFileName),
"<not a valid envelope>");
var result = DeathRattlePrelude.TryCapture(
stateDirectory: tmp.Path,
previousExitType: "reboot_kill",
isWhiteGloveResume: false,
logger: NewLogger(tmp));
Assert.Null(result);
}
// ============================================================================
// Path-layout contract — must match orchestrator's hardcoded path
// ============================================================================
[Fact]
public void Reads_from_path_layout_matching_orchestrator()
{
// EnrollmentOrchestrator hardcodes Path.Combine(_stateDirectory,
// "snapshot.json") at line 309. If anyone renames the file or moves it under
// a subfolder on either side without updating the other, the death-rattle
// would silently miss every snapshot. This test pins the exact filename
// contract — a change of the literal "snapshot.json" requires the test to
// be updated alongside both producers.
using var tmp = new TempDirectory();
var expectedPath = Path.Combine(tmp.Path, "snapshot.json");
new SnapshotPersistence(expectedPath).Save(BuildState(stepIndex: 99));
var result = DeathRattlePrelude.TryCapture(
stateDirectory: tmp.Path,
previousExitType: "reboot_kill",
isWhiteGloveResume: false,
logger: NewLogger(tmp));
Assert.NotNull(result);
Assert.Equal(99, result!.StepIndex);
// Belt + suspenders: the file at the literal path is what we read.
Assert.True(File.Exists(expectedPath));
}
// ============================================================================
// Non-mutation contract — orchestrator.Start() must see the snapshot intact
// ============================================================================
[Fact]
public void Does_NOT_modify_or_quarantine_the_snapshot_file_on_success()
{
// Critical for ordering: orchestrator.Start runs Snapshot.Load() right after
// the prelude. If the prelude moves / overwrites / quarantines the file,
// Start would see no snapshot and start from scratch — losing all recovery
// state. Pin via byte-level equality + post-prelude Load() round-trip.
using var tmp = new TempDirectory();
var path = SaveSnapshot(tmp.Path);
var bytesBefore = File.ReadAllBytes(path);
var result = DeathRattlePrelude.TryCapture(
stateDirectory: tmp.Path,
previousExitType: "reboot_kill",
isWhiteGloveResume: false,
logger: NewLogger(tmp));
Assert.NotNull(result);
var bytesAfter = File.ReadAllBytes(path);
Assert.Equal(bytesBefore, bytesAfter);
// The file is also still loadable through the orchestrator-style instance API.
var viaInstance = new SnapshotPersistence(path).Load();
Assert.NotNull(viaInstance);
Assert.Equal(result!.Stage, viaInstance!.Stage);
Assert.Equal(result.StepIndex, viaInstance.StepIndex);
// No quarantine directory has been created either.
Assert.False(Directory.Exists(Path.Combine(tmp.Path, ".quarantine")));
}
[Fact]
public void Does_NOT_modify_or_quarantine_the_snapshot_file_on_corruption()
{
// Even when TryReadRaw fails the prelude must NEVER touch the file —
// orchestrator.Start owns the quarantine decision. If the prelude
// quarantined corrupt snapshots, Start would see a missing file and
// skip its own quarantine path → loss of forensic evidence.
using var tmp = new TempDirectory();
var path = Path.Combine(tmp.Path, DeathRattlePrelude.SnapshotFileName);
File.WriteAllText(path, "<corrupt>");
var bytesBefore = File.ReadAllBytes(path);
var result = DeathRattlePrelude.TryCapture(
stateDirectory: tmp.Path,
previousExitType: "reboot_kill",
isWhiteGloveResume: false,
logger: NewLogger(tmp));
Assert.Null(result);
Assert.True(File.Exists(path));
Assert.Equal(bytesBefore, File.ReadAllBytes(path));
Assert.False(Directory.Exists(Path.Combine(tmp.Path, ".quarantine")));
}
// ============================================================================
// Argument validation
// ============================================================================
[Fact]
public void Throws_for_null_logger()
{
using var tmp = new TempDirectory();
Assert.Throws<ArgumentNullException>(() => DeathRattlePrelude.TryCapture(
stateDirectory: tmp.Path,
previousExitType: "reboot_kill",
isWhiteGloveResume: false,
logger: null!));
}
[Theory]
[InlineData(null)]
[InlineData("")]
public void Throws_for_null_or_empty_stateDirectory(string? stateDirectory)
{
using var tmp = new TempDirectory();
Assert.Throws<ArgumentException>(() => DeathRattlePrelude.TryCapture(
stateDirectory: stateDirectory!,
previousExitType: "reboot_kill",
isWhiteGloveResume: false,
logger: NewLogger(tmp)));
}
}
}
@@ -0,0 +1,108 @@
#nullable enable
using System;
using System.IO;
using AutopilotMonitor.Agent.V2.Core.Logging;
using AutopilotMonitor.DecisionCore.State;
namespace AutopilotMonitor.Agent.V2.Core.Persistence
{
/// <summary>
/// Death-Rattle prelude (Plan §B — Edge-Triggered State Snapshots, 2026-05-03).
/// Captures the prior agent run's last persisted <see cref="DecisionState"/> BEFORE the
/// new run's <c>EnrollmentOrchestrator.Start()</c> runs the recovery pipeline. Returns
/// <c>null</c> when the previous exit was clean, when this is a planned WhiteGlove
/// Part-2 resume, when the snapshot is missing or corrupt, or when any I/O error
/// occurs — the caller emits the <c>prior_run_died_with_state</c> event only when
/// this returns non-null.
/// <para>
/// Extracted from <c>AgentRuntimeHost</c> so the gate logic + ordering contract are
/// directly testable. Three regress classes the host wiring is fragile against:
/// </para>
/// <list type="number">
/// <item><c>TryReadRaw</c> moves to after <c>orchestrator.Start()</c> → snapshot
/// either overwritten by the first reducer save or quarantined → we read the wrong
/// state. Pin via the "no-mutation" contract test.</item>
/// <item>WhiteGlove Part-2 resume no longer skipped → false-positive death-rattle
/// for a planned exit. Pin via the WG-skip test.</item>
/// <item><c>clean</c> / <c>first_run</c> exits start emitting → telemetry noise on
/// every successful previous run. Pin via the clean-exit-table test.</item>
/// </list>
/// </summary>
public static class DeathRattlePrelude
{
// The orchestrator hardcodes this exact filename inside _stateDirectory
// (EnrollmentOrchestrator.cs:309 — `Path.Combine(_stateDirectory, "snapshot.json")`).
// Any rename on either side without updating both is a bit-level path-layout drift
// that DeathRattlePreludeTests.Reads_from_path_layout_matching_orchestrator catches.
internal const string SnapshotFileName = "snapshot.json";
/// <summary>
/// Returns the prior run's persisted snapshot if all gates pass, else <c>null</c>.
/// </summary>
/// <param name="stateDirectory">
/// Same directory the orchestrator's <c>SnapshotPersistence</c> reads from / writes
/// to. Must be non-empty.
/// </param>
/// <param name="previousExitType">
/// One of <c>first_run</c> / <c>clean</c> / <c>exception_crash</c> / <c>hard_kill</c>
/// / <c>reboot_kill</c> as classified by <c>Program.DetectPreviousExit</c>.
/// Case-insensitive on principle.
/// </param>
/// <param name="isWhiteGloveResume">
/// <c>true</c> when this run is the Part-2 continuation of a sealed WhiteGlove flow.
/// The "death" of the Part-1 run is a planned reboot, not an unclean exit, so no
/// attestation is warranted.
/// </param>
/// <param name="logger">Mandatory — used for telemetry-friendly Info / Debug / Warning lines.</param>
public static DecisionState? TryCapture(
string stateDirectory,
string? previousExitType,
bool isWhiteGloveResume,
AgentLogger logger)
{
if (logger == null) throw new ArgumentNullException(nameof(logger));
if (string.IsNullOrEmpty(stateDirectory))
throw new ArgumentException("stateDirectory is mandatory.", nameof(stateDirectory));
if (isWhiteGloveResume) return null;
if (!IsUncleanExit(previousExitType)) return null;
try
{
var snapshotPath = Path.Combine(stateDirectory, SnapshotFileName);
var prior = SnapshotPersistence.TryReadRaw(snapshotPath);
if (prior != null)
{
logger.Info(
$"Death-rattle: prior snapshot loaded (Stage={prior.Stage}, " +
$"StepIndex={prior.StepIndex}, exitType={previousExitType}).");
}
else
{
logger.Debug(
$"Death-rattle: no prior snapshot to attest (path missing or unreadable, exitType={previousExitType}).");
}
return prior;
}
catch (Exception ex)
{
logger.Warning($"Death-rattle: prior snapshot read failed: {ex.Message}");
return null;
}
}
/// <summary>
/// Gate: which previous-exit classifications warrant a death-rattle. The set is
/// intentionally small — <c>first_run</c> / <c>clean</c> are planned exits with no
/// state worth re-attesting. Case-insensitive on principle to be robust against any
/// future capitalization drift in the producer.
/// </summary>
public static bool IsUncleanExit(string? exitType)
{
return exitType != null
&& (StringComparer.OrdinalIgnoreCase.Equals(exitType, "reboot_kill")
|| StringComparer.OrdinalIgnoreCase.Equals(exitType, "hard_kill")
|| StringComparer.OrdinalIgnoreCase.Equals(exitType, "exception_crash"));
}
}
}
@@ -1,5 +1,4 @@
using System;
using System.IO;
using System.Threading;
using System.Threading.Tasks;
using AutopilotMonitor.Agent.V2.Core.Configuration;
@@ -201,41 +200,18 @@ namespace AutopilotMonitor.Agent.V2.Runtime
logger: logger);
auth.AuthFailureTracker.ThresholdExceeded += authThresholdHandler;
// Death-Rattle (Plan §B, 2026-05-03): if the previous run died unclean
// AND its persisted snapshot is still readable, capture it as an
// in-memory draft BEFORE orchestrator.Start runs. Start triggers the
// recovery pipeline which may quarantine the snapshot or have the first
// reducer save overwrite it; reading "what the dead run last knew"
// through the orchestrator-owned SnapshotPersistence after Start would
// either return the post-Start state (wrong) or null. The static
// TryReadRaw is quarantine-free and shares the parse path with Load(),
// so failure modes match. Skip on WhiteGlove Part-2 resume — the
// "dying" run there is the Part-1 sealing run, which is a planned
// exit, not a death.
DecisionState priorStateForDeathRattle = null;
if (!isWhiteGloveResume && IsUncleanExit(previousExit?.ExitType))
{
try
{
var snapshotPath = Path.Combine(stateSubdir, "snapshot.json");
priorStateForDeathRattle = SnapshotPersistence.TryReadRaw(snapshotPath);
if (priorStateForDeathRattle != null)
{
logger.Info(
$"Death-rattle: prior snapshot loaded (Stage={priorStateForDeathRattle.Stage}, " +
$"StepIndex={priorStateForDeathRattle.StepIndex}, exitType={previousExit?.ExitType}).");
}
else
{
logger.Debug(
$"Death-rattle: no prior snapshot to attest (path missing or unreadable, exitType={previousExit?.ExitType}).");
}
}
catch (Exception ex)
{
logger.Warning($"Death-rattle: prior snapshot read failed: {ex.Message}");
}
}
// Death-Rattle (Plan §B, 2026-05-03): capture the prior run's last
// persisted snapshot BEFORE orchestrator.Start runs. Start triggers
// the recovery pipeline which may quarantine the snapshot or have the
// first reducer save overwrite it; reading after Start would either
// return the post-Start state (wrong) or null. The gate + path + read
// live in DeathRattlePrelude so they are directly testable in
// isolation — see DeathRattlePreludeTests.
DecisionState priorStateForDeathRattle = DeathRattlePrelude.TryCapture(
stateDirectory: stateSubdir,
previousExitType: previousExit?.ExitType,
isWhiteGloveResume: isWhiteGloveResume,
logger: logger);
try
{
@@ -525,20 +501,5 @@ namespace AutopilotMonitor.Agent.V2.Runtime
return requested;
}
/// <summary>
/// Death-Rattle gate (Plan §B, 2026-05-03). The classifications produced by
/// <see cref="Program.DetectPreviousExit"/> are: <c>first_run</c> / <c>clean</c>
/// (planned exits — no death-rattle), <c>exception_crash</c> / <c>hard_kill</c>
/// / <c>reboot_kill</c> (unclean — the prior run probably had un-reported state
/// worth attesting). Case-insensitive on principle to be robust against any
/// future capitalization drift in the producer.
/// </summary>
private static bool IsUncleanExit(string exitType)
{
return exitType != null
&& (StringComparer.OrdinalIgnoreCase.Equals(exitType, "reboot_kill")
|| StringComparer.OrdinalIgnoreCase.Equals(exitType, "hard_kill")
|| StringComparer.OrdinalIgnoreCase.Equals(exitType, "exception_crash"));
}
}
}
@@ -1 +1 @@
552
555