From 52aea2f4b054a2aa3334ffd7b78c7217c96d947c Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Fri, 7 Aug 2026 22:12:30 +0000 Subject: [PATCH 1/4] fix(reminders): retain failed one-shot executions --- Directory.Packages.props | 2 +- .../.system/files/netclaw-operations/SKILL.md | 2 +- .../references/scheduling.md | 32 +- .../.openspec.yaml | 2 + .../design.md | 75 +++++ .../proposal.md | 32 ++ .../specs/netclaw-scheduling/spec.md | 93 ++++++ .../specs/reminder-execution-history/spec.md | 17 + .../reliable-one-shot-reminder-retry/tasks.md | 25 ++ .../Reminders/ReminderDefinitionStoreTests.cs | 67 ++++ .../Reminders/ReminderExecutionActorTests.cs | 78 ++++- .../Reminders/ReminderManagerActorTests.cs | 175 +++++++++-- .../Channels/SessionPipelineHandle.cs | 54 +++- .../Hosting/NetclawAkkaHostingExtensions.cs | 14 +- .../Reminders/ActiveExecutionTracker.cs | 33 +- .../Reminders/ReminderExecutionActor.cs | 152 ++++++--- .../Reminders/ReminderManagerActor.cs | 293 +++++++++++++----- .../Reminders/ReminderProtocol.cs | 50 ++- src/Netclaw.Cli/Reminder/ReminderCommand.cs | 24 +- src/Netclaw.Daemon/Program.cs | 6 +- .../ReminderEndpointRouteBuilderExtensions.cs | 22 +- .../Webhooks/WebhookExecutionActor.cs | 14 +- 22 files changed, 1074 insertions(+), 188 deletions(-) create mode 100644 openspec/changes/reliable-one-shot-reminder-retry/.openspec.yaml create mode 100644 openspec/changes/reliable-one-shot-reminder-retry/design.md create mode 100644 openspec/changes/reliable-one-shot-reminder-retry/proposal.md create mode 100644 openspec/changes/reliable-one-shot-reminder-retry/specs/netclaw-scheduling/spec.md create mode 100644 openspec/changes/reliable-one-shot-reminder-retry/specs/reminder-execution-history/spec.md create mode 100644 openspec/changes/reliable-one-shot-reminder-retry/tasks.md diff --git a/Directory.Packages.props b/Directory.Packages.props index 80a8dc120..ebd059d7f 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -5,7 +5,7 @@ 1.5.70 1.5.70 - 0.6.0 + 0.7.0 1.17.0 0.17.10 3.20.1 diff --git a/feeds/skills/.system/files/netclaw-operations/SKILL.md b/feeds/skills/.system/files/netclaw-operations/SKILL.md index c67a87120..e2481b02b 100644 --- a/feeds/skills/.system/files/netclaw-operations/SKILL.md +++ b/feeds/skills/.system/files/netclaw-operations/SKILL.md @@ -3,7 +3,7 @@ name: netclaw-operations description: "REQUIRED when the user asks about scheduling, reminders, cron jobs, timers, background jobs, diagnostics, troubleshooting, MCP tools, daemon health, identity updates, or Netclaw capabilities and self-maintenance." metadata: author: netclaw - version: "2.40.0" + version: "2.41.0" --- # Netclaw Operations diff --git a/feeds/skills/.system/files/netclaw-operations/references/scheduling.md b/feeds/skills/.system/files/netclaw-operations/references/scheduling.md index cf65fd583..066ad7ef6 100644 --- a/feeds/skills/.system/files/netclaw-operations/references/scheduling.md +++ b/feeds/skills/.system/files/netclaw-operations/references/scheduling.md @@ -62,6 +62,19 @@ Reminders that hit 5 consecutive execution failures are auto-disabled with a `ReminderAutoDisabled` critical alert. The definition stays on disk so the operator can diagnose and re-enable after fixing the root cause. +A known execution or delivery failure starts the Akka.Reminders retry policy. +The retry uses bounded backoff and the same durable occurrence identity. A +successful attempt resets the consecutive failure count. + +A one-shot reminder stays enabled while an occurrence can retry. A successful +one-shot becomes disabled with a `Completed` outcome. A poison one-shot becomes +disabled with a `Failed` outcome. Both definitions and their history remain +available until an operator uses the permanent delete command. + +Each attempt has a 20-minute inactivity limit and a one-hour absolute limit. +The durable acknowledgement lease is 70 minutes. A daemon crash therefore lets +Akka.Reminders retry the occurrence after the lease expires. + **Failure visibility.** When a reminder execution fails for any reason — including the 20-minute stall backstop that recovers a wedged run — the failure is posted as a plain-language notice to the reminder's **destination channel** (for @@ -69,20 +82,21 @@ as a plain-language notice to the reminder's **destination channel** (for reminder's output. This is bounded by the auto-disable threshold (at most a few notices plus the disabled notice), not the unbounded skip stream. -A *skipped* fire (one that arrives while the prior execution is still running) is -**not** posted to the channel — it would be too noisy — but it is counted and -surfaced by the status command: +A second fire waits in the deferred queue while the prior execution runs. +Netclaw counts this event but does not post it to the channel. The status command +shows the count: ``` netclaw reminder status ``` -`status` shows, per reminder: whether it's enabled, whether an execution is in -flight right now, when it next fires, the consecutive-failure count, the -skipped-fire count (since daemon start), and recent run history. Reach for it -when a reminder seems to have silently stopped doing its job — a high skip count -means a prior run is wedged (it should self-recover within ~20 minutes), and a -rising failure count points at a misconfigured or broken reminder. +`status` shows the enabled state, the terminal outcome, and current execution +state. It also shows the next fire, consecutive failures, deferred overlap count, +and recent history. For one-shots, it shows the durable occurrence state, attempt +count, next retry time, and last failure reason. + +Use this command when a reminder stops its expected work. A failure count that +increases usually means that the reminder or its delivery target is not healthy. If `audience` is omitted during conversational scheduling, the reminder inherits the audience of the channel/session that created it. A reminder cannot be diff --git a/openspec/changes/reliable-one-shot-reminder-retry/.openspec.yaml b/openspec/changes/reliable-one-shot-reminder-retry/.openspec.yaml new file mode 100644 index 000000000..878dc3156 --- /dev/null +++ b/openspec/changes/reliable-one-shot-reminder-retry/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-08-07 diff --git a/openspec/changes/reliable-one-shot-reminder-retry/design.md b/openspec/changes/reliable-one-shot-reminder-retry/design.md new file mode 100644 index 000000000..e572b0596 --- /dev/null +++ b/openspec/changes/reliable-one-shot-reminder-retry/design.md @@ -0,0 +1,75 @@ +## Context + +Akka.Reminders already persists each occurrence attempt, deadline, failure reason, and terminal state. Netclaw acknowledges channel and no-delivery reminders before their LLM session completes. + +A failed one-shot then has no active Akka occurrence. Reconciliation treats that absence as completion and deletes the definition and history. + +## Goals / Non-Goals + +**Goals:** + +- Use Akka.Reminders as the occurrence retry source of truth. +- Preserve failed one-shots until retry success or a terminal failure. +- Preserve the separate Netclaw poison threshold for the complete reminder. +- Keep old reminder JSON files and the Akka.Reminders 0.6.0 schema compatible. + +**Non-Goals:** + +- Add a recurring catch-up queue. +- Add a durable ingress queue for all session messages. +- Change reminder trust or tool-policy derivation. + +## Decisions + +### Akka.Reminders owns occurrence retry state + +Netclaw uses the entity-bound `IReminderClient.NackAsync` method for a known failure. It uses `GetOccurrenceStatusAsync` for retry and terminal diagnostics. + +Netclaw does not copy an occurrence attempt count or retry timestamp into its definition file. + +### Netclaw owns reminder-level poison state + +Netclaw persists `ConsecutiveFailures` in each reminder definition. Each failed execution attempt increments this value, and a success resets it. + +This value spans recurring occurrences. Akka.Reminders resets its attempt count for each new occurrence. + +### Every delivery mode delays the acknowledgment + +`ReminderManagerActor` passes the envelope to `ReminderExecutionActor` for all delivery kinds. The child acknowledges only after execution and required delivery succeed. + +The child sends a negative acknowledgement after a known failure. An actor crash leaves the occurrence unacknowledged, so the library timeout remains the final recovery path. + +### One-shot completion uses a soft delete + +A successful one-shot sets `Enabled` to false and records `TerminalOutcome.Completed`. A poison or terminal one-shot records `TerminalOutcome.Failed`. + +The explicit delete command remains the only normal hard-delete path. It also deletes the history file. + +### Reconciliation uses durable state + +Reconciliation never uses a past fire time or a missing active schedule as proof of success. It retains disabled one-shots and restores an enabled one-shot when durable state permits another attempt. + +### Timeouts remain bounded + +Netclaw sets the Akka acknowledgment timeout to 70 minutes. The execution actor applies a one-hour absolute attempt limit and keeps its 20-minute inactivity limit. + +Known failures use negative acknowledgement and do not wait for the acknowledgment timeout. + +## Risks / Trade-offs + +- **A daemon crash can delay retry for 70 minutes.** The long lease prevents duplicate LLM work during a valid one-hour attempt. +- **At-least-once delivery can duplicate work.** The occurrence identity remains `(Entity, Key, DueTimeUtc)` and session identifiers use that stable due time. +- **A custom Akka storage provider can lack status queries.** Netclaw uses the official SQLite provider and fails loudly if the capability is absent. +- **Old JSON files lack the new fields.** Serializer defaults preserve active state and a zero failure count. + +## Migration Plan + +1. Release Akka.Reminders 0.7.0 with the additive delivery-control API. +2. Upgrade Netclaw to both 0.7.0 packages. +3. Load old reminder JSON files with default values. +4. Keep the existing SQLite schema without a migration. +5. Roll back Netclaw by restoring the prior binary. The added JSON fields remain harmless to older readers. + +## Open Questions + +None. diff --git a/openspec/changes/reliable-one-shot-reminder-retry/proposal.md b/openspec/changes/reliable-one-shot-reminder-retry/proposal.md new file mode 100644 index 000000000..0bcea8281 --- /dev/null +++ b/openspec/changes/reliable-one-shot-reminder-retry/proposal.md @@ -0,0 +1,32 @@ +## Why + +PRD-008 requires durable failure records and an automatic pause after repeated failures. Netclaw now deletes a failed one-shot before Akka.Reminders can retry it. + +## What Changes + +- Netclaw will acknowledge every reminder only after successful execution and required delivery. +- Netclaw will report a known failure through the Akka.Reminders negative acknowledgement API. +- A failed one-shot will remain enabled while another occurrence attempt is pending. +- A completed or terminally failed one-shot will use a soft delete. +- Netclaw will persist its reminder-level consecutive failure count. +- Reconciliation will use durable occurrence state and will never infer success from a past due time. +- Reminder status output will show the durable occurrence attempt and terminal outcome. + +## Capabilities + +### New Capabilities + +None. + +### Modified Capabilities + +- `netclaw-scheduling`: Change acknowledgement, retry, poison-reminder, one-shot retention, and reconciliation requirements. +- `reminder-execution-history`: Retain execution history for soft-deleted one-shot reminders. + +## Impact + +- **In scope:** PRD-008 reminder execution, reminder status, reconciliation, the definition store, and Akka.Reminders 0.7.0 integration. +- **Out of scope:** A catch-up queue for recurring occurrences and a general durable ingress queue for all sessions. +- **Security:** The change keeps the current trust context and tool policy for every retry. +- **Operations:** Operators can inspect failed one-shots and retry state after a daemon restart. +- **Compatibility:** Existing reminder JSON files load with default values. The Akka.Reminders 0.6.0 database schema remains valid. diff --git a/openspec/changes/reliable-one-shot-reminder-retry/specs/netclaw-scheduling/spec.md b/openspec/changes/reliable-one-shot-reminder-retry/specs/netclaw-scheduling/spec.md new file mode 100644 index 000000000..6076b5761 --- /dev/null +++ b/openspec/changes/reliable-one-shot-reminder-retry/specs/netclaw-scheduling/spec.md @@ -0,0 +1,93 @@ +## ADDED Requirements + +### Requirement: Execution outcome controls occurrence acknowledgement + +Netclaw SHALL pass the Akka.Reminders envelope to every reminder execution. Netclaw SHALL acknowledge an occurrence only after successful execution and required delivery. + +Netclaw SHALL send a negative acknowledgement after a known execution or delivery failure. The negative acknowledgement SHALL use the library retry budget. + +#### Scenario: Channel execution fails before delivery + +- **GIVEN** an enabled channel reminder occurrence is awaiting acknowledgement +- **WHEN** its session fails before required delivery succeeds +- **THEN** Netclaw sends a negative acknowledgement with the failure reason +- **AND** Netclaw does not send a successful acknowledgement +- **AND** Akka.Reminders persists the next attempt or a terminal state + +#### Scenario: Execution and required delivery succeed + +- **GIVEN** an enabled reminder occurrence is awaiting acknowledgement +- **WHEN** execution and required delivery succeed +- **THEN** Netclaw acknowledges the exact occurrence +- **AND** Akka.Reminders records `Delivered` + +### Requirement: Reminder-level poison state is durable + +Netclaw SHALL persist a consecutive execution failure count in the reminder definition. Each failed attempt SHALL increment the count, and a successful attempt SHALL reset it. + +Netclaw SHALL disable the complete reminder when the count reaches `FailurePauseThreshold`. This count SHALL remain separate from the Akka.Reminders per-occurrence attempt count. + +#### Scenario: Restart preserves the poison count + +- **GIVEN** a reminder has three consecutive failed attempts +- **WHEN** the daemon restarts +- **THEN** reminder status reports three consecutive failures +- **AND** the next failed attempt increments the count to four + +#### Scenario: Success resets the poison count + +- **GIVEN** a reminder has one or more consecutive failed attempts +- **WHEN** a later attempt succeeds +- **THEN** Netclaw persists a zero consecutive failure count + +#### Scenario: Fifth failure disables the complete reminder + +- **GIVEN** a reminder has four consecutive failed attempts +- **WHEN** the next attempt fails +- **THEN** Netclaw disables the reminder +- **AND** Netclaw records a failed terminal outcome +- **AND** Netclaw cancels future occurrences for the complete reminder + +### Requirement: One-shot reminders use soft deletion + +Netclaw SHALL retain a one-shot definition after success or terminal failure. Netclaw SHALL disable the definition and record its terminal outcome. + +Only an explicit delete command SHALL remove the definition and history. + +#### Scenario: Successful one-shot remains inspectable + +- **GIVEN** a one-shot reminder succeeds +- **WHEN** Netclaw completes its acknowledgement +- **THEN** Netclaw disables the definition with outcome `Completed` +- **AND** an all-reminders query returns the definition + +#### Scenario: Failed one-shot remains enabled for retry + +- **GIVEN** a one-shot attempt fails below the poison threshold +- **WHEN** Akka.Reminders schedules another attempt +- **THEN** Netclaw keeps the definition enabled +- **AND** reminder status shows the durable attempt state + +#### Scenario: Reconciliation retains a past one-shot + +- **GIVEN** a one-shot has a past fire time +- **WHEN** reconciliation finds no active schedule +- **THEN** reconciliation does not delete the definition or history +- **AND** reconciliation uses durable occurrence state to select restoration or a terminal soft delete + +### Requirement: Reminder attempts have bounded acknowledgement leases + +Netclaw SHALL use a one-hour absolute execution limit and a 70-minute Akka.Reminders acknowledgment timeout. It SHALL retain the 20-minute inactivity limit. + +#### Scenario: Valid long execution completes within the lease + +- **GIVEN** a reminder execution produces activity and completes within one hour +- **WHEN** required delivery succeeds +- **THEN** Netclaw acknowledges the occurrence before its 70-minute deadline + +#### Scenario: Execution reaches the absolute limit + +- **GIVEN** a reminder execution remains active for one hour +- **WHEN** the absolute limit expires +- **THEN** Netclaw stops the attempt +- **AND** Netclaw sends a negative acknowledgement diff --git a/openspec/changes/reliable-one-shot-reminder-retry/specs/reminder-execution-history/spec.md b/openspec/changes/reliable-one-shot-reminder-retry/specs/reminder-execution-history/spec.md new file mode 100644 index 000000000..c61208697 --- /dev/null +++ b/openspec/changes/reliable-one-shot-reminder-retry/specs/reminder-execution-history/spec.md @@ -0,0 +1,17 @@ +## ADDED Requirements + +### Requirement: Soft deletion retains reminder history + +Netclaw SHALL retain execution history when it soft-deletes a completed or failed one-shot. Only an explicit delete command SHALL remove the history file. + +#### Scenario: Completed one-shot retains history + +- **GIVEN** a one-shot has a successful execution record +- **WHEN** Netclaw disables it with outcome `Completed` +- **THEN** the definition and history file remain present + +#### Scenario: Failed one-shot retains history + +- **GIVEN** a one-shot reaches its poison threshold +- **WHEN** Netclaw disables it with outcome `Failed` +- **THEN** all failure records remain available through reminder history diff --git a/openspec/changes/reliable-one-shot-reminder-retry/tasks.md b/openspec/changes/reliable-one-shot-reminder-retry/tasks.md new file mode 100644 index 000000000..eb763dcea --- /dev/null +++ b/openspec/changes/reliable-one-shot-reminder-retry/tasks.md @@ -0,0 +1,25 @@ +## 1. Dependency and persistence + +- [x] 1.1 Upgrade both Akka.Reminders packages to version 0.7.0 and configure the 70-minute acknowledgement timeout. +- [x] 1.2 Add backward-compatible terminal outcome and consecutive failure fields to reminder definitions. +- [x] 1.3 Add old-shape load and round-trip tests for reminder JSON files. + +## 2. Execution and retry + +- [x] 2.1 Pass the durable envelope to every reminder execution mode. +- [x] 2.2 Acknowledge success and negatively acknowledge known execution or delivery failures. +- [x] 2.3 Add the one-hour absolute attempt limit and session-termination failure detection. +- [x] 2.4 Use the occurrence due time for stable retry session identity. + +## 3. Lifecycle and reconciliation + +- [x] 3.1 Persist each reminder-level failure increment and each success reset. +- [x] 3.2 Retain retryable one-shots and soft-delete completed or terminal one-shots. +- [x] 3.3 Remove automatic hard deletion from reconciliation and use durable occurrence status. +- [x] 3.4 Expose occurrence retry and terminal details through reminder status. + +## 4. Proof and guidance + +- [x] 4.1 Add actor tests for retry, later success, poison pause, restart state, and reconciliation retention. +- [x] 4.2 Update the `netclaw-operations` system skill and its version. +- [x] 4.3 Run focused tests, the full affected suites, evals, Slopwatch, and file-header verification. diff --git a/src/Netclaw.Actors.Tests/Reminders/ReminderDefinitionStoreTests.cs b/src/Netclaw.Actors.Tests/Reminders/ReminderDefinitionStoreTests.cs index 0a6b6728f..8734fb4d4 100644 --- a/src/Netclaw.Actors.Tests/Reminders/ReminderDefinitionStoreTests.cs +++ b/src/Netclaw.Actors.Tests/Reminders/ReminderDefinitionStoreTests.cs @@ -170,6 +170,73 @@ public void Current_reminder_with_trust_fields_roundtrips_exact_values() Assert.Equal(TrustBoundary.Personal, loaded.Boundary); Assert.Equal(id, loaded.Id.Value); Assert.Equal("Round-trip check", loaded.Title); + Assert.Equal(0, loaded.ConsecutiveFailures); + Assert.Null(loaded.TerminalOutcome); + } + + [Fact] + public void Reminder_failure_state_roundtrips() + { + var store = new ReminderDefinitionStore(_paths); + var now = TimeProvider.System.GetUtcNow(); + var definition = new ReminderDefinition + { + Id = new ReminderId("roundtrip-failure-state"), + Title = "Failure state check", + Instructions = "Do the thing.", + Delivery = new ReminderDelivery { Kind = DeliveryKind.None }, + Schedule = new ReminderSchedule + { + Type = ReminderScheduleType.OneShot, + FireAt = now.AddHours(1) + }, + Audience = TrustAudience.Personal, + Boundary = TrustBoundary.Personal, + Enabled = false, + ConsecutiveFailures = 5, + TerminalOutcome = ReminderTerminalOutcome.Failed, + CreatedAt = now, + UpdatedAt = now + }; + + store.Save(definition); + + var loaded = new ReminderDefinitionStore(_paths).Get(definition.Id); + + Assert.NotNull(loaded); + Assert.Equal(5, loaded.ConsecutiveFailures); + Assert.Equal(ReminderTerminalOutcome.Failed, loaded.TerminalOutcome); + } + + [Fact] + public void Definition_without_failure_fields_loads_with_active_defaults() + { + var reminderId = "old-failure-shape"; + var filePath = Path.Combine( + _paths.RemindersDirectory, + $"{Uri.EscapeDataString(reminderId)}.json"); + const string json = """ + { + "id": "old-failure-shape", + "title": "Old shape", + "schedule": { "type": "OneShot", "fireAtMs": 1800000000000 }, + "instructions": "Check status.", + "delivery": { "kind": "None" }, + "enabled": true, + "audience": "Personal", + "boundary": "personal", + "createdAtMs": 1700000000000, + "updatedAtMs": 1700000000000 + } + """; + File.WriteAllText(filePath, json); + + var loaded = new ReminderDefinitionStore(_paths).Get(new ReminderId(reminderId)); + + Assert.NotNull(loaded); + Assert.Equal(0, loaded!.ConsecutiveFailures); + Assert.Null(loaded.TerminalOutcome); + Assert.True(loaded.Enabled); } /// diff --git a/src/Netclaw.Actors.Tests/Reminders/ReminderExecutionActorTests.cs b/src/Netclaw.Actors.Tests/Reminders/ReminderExecutionActorTests.cs index 8851f80da..c3d2beaf6 100644 --- a/src/Netclaw.Actors.Tests/Reminders/ReminderExecutionActorTests.cs +++ b/src/Netclaw.Actors.Tests/Reminders/ReminderExecutionActorTests.cs @@ -254,10 +254,12 @@ public async Task Mode_A_wedged_session_is_failed_by_stall_backstop_releasing_th { // Emits one non-terminal output, then goes silent forever — no // TurnCompleted/Error ever arrives. - var pipeline = new ScriptedSessionPipeline(sessionId => - [ - new TextOutput("Working on it...") { SessionId = sessionId } - ]); + var pipeline = new ScriptedSessionPipeline( + sessionId => + [ + new TextOutput("Working on it...") { SessionId = sessionId } + ], + keepOutputOpen: true); var definition = CreateDefinition("mode-a-stall"); var probe = CreateTestProbe(); @@ -278,6 +280,62 @@ public async Task Mode_A_wedged_session_is_failed_by_stall_backstop_releasing_th } } + [Fact] + public async Task Execution_fails_when_output_stream_ends_without_terminal_output() + { + var pipeline = new ScriptedSessionPipeline(sessionId => + [ + new TextOutput("Partial output") { SessionId = sessionId } + ]); + var definition = CreateDefinition("stream-ended"); + var probe = CreateTestProbe(); + Sys.ActorOf( + Props.Create(() => new ParentProxy(probe.Ref, definition, pipeline, _historyStore)), + "exec-stream-ended"); + + var completed = await probe.ExpectMsgAsync( + TimeSpan.FromSeconds(5), + cancellationToken: TestContext.Current.CancellationToken); + + Assert.False(completed.Success); + Assert.Contains("ended", completed.ErrorMessage!, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public async Task Execution_fails_at_absolute_attempt_limit_despite_output_activity() + { + var originalAttemptTimeout = ReminderExecutionActor.ExecutionAttemptTimeout; + var originalStallTimeout = ReminderExecutionActor.ExecutionStallTimeout; + ReminderExecutionActor.ExecutionAttemptTimeout = TimeSpan.FromMilliseconds(250); + ReminderExecutionActor.ExecutionStallTimeout = TimeSpan.FromSeconds(5); + try + { + var pipeline = new ScriptedSessionPipeline( + sessionId => + [ + new TextOutput("Still active") { SessionId = sessionId } + ], + keepOutputOpen: true); + var definition = CreateDefinition("absolute-limit"); + var probe = CreateTestProbe(); + Sys.ActorOf( + Props.Create(() => new ParentProxy(probe.Ref, definition, pipeline, _historyStore)), + "exec-absolute-limit"); + + var completed = await probe.ExpectMsgAsync( + TimeSpan.FromSeconds(5), + cancellationToken: TestContext.Current.CancellationToken); + + Assert.False(completed.Success); + Assert.Contains("exceeded", completed.ErrorMessage!, StringComparison.OrdinalIgnoreCase); + } + finally + { + ReminderExecutionActor.ExecutionAttemptTimeout = originalAttemptTimeout; + ReminderExecutionActor.ExecutionStallTimeout = originalStallTimeout; + } + } + private static ReminderDefinition CreateDefinition(string id) { var now = TimeProvider.System.GetUtcNow(); @@ -346,7 +404,8 @@ public Task SendFeedbackAndWaitAsync(IWithSessionId feedback, } private sealed class ScriptedSessionPipeline( - Func> outputFactory) : ISessionPipeline + Func> outputFactory, + bool keepOutputOpen = false) : ISessionPipeline { private readonly TaskCompletionSource _inputCaptured = new(TaskCreationOptions.RunContinuationsAsynchronously); @@ -371,7 +430,7 @@ public Task CreateAsync( .MapMaterializedValue(_ => NotUsed.Instance); var outputs = outputFactory(sessionId).ToList(); - var output = Source.UnfoldAsync(0, async state => + Source output = Source.UnfoldAsync(0, async state => { if (state == 0) await _inputCaptured.Task.ConfigureAwait(false); @@ -383,6 +442,13 @@ public Task CreateAsync( }) .Via(killSwitch.Flow()); + if (keepOutputOpen) + { + output = output.Concat( + Source.Maybe() + .MapMaterializedValue(_ => NotUsed.Instance)); + } + return Task.FromResult(new MaterializedSession(captureInputSink, output, killSwitch)); } diff --git a/src/Netclaw.Actors.Tests/Reminders/ReminderManagerActorTests.cs b/src/Netclaw.Actors.Tests/Reminders/ReminderManagerActorTests.cs index 57beba5b3..f584f3753 100644 --- a/src/Netclaw.Actors.Tests/Reminders/ReminderManagerActorTests.cs +++ b/src/Netclaw.Actors.Tests/Reminders/ReminderManagerActorTests.cs @@ -9,6 +9,7 @@ using Akka.Persistence.Hosting; using Akka.Reminders; using Akka.Reminders.Sharding; +using Akka.Streams; using Microsoft.Extensions.Time.Testing; using Netclaw.Actors.Channels; using Netclaw.Actors.Hosting; @@ -28,6 +29,8 @@ public class ReminderManagerActorTests : TestKit private readonly FakeTimeProvider _timeProvider = new(TimeProvider.System.GetUtcNow()); private ReminderDefinitionStore _definitionStore = null!; private TestNotificationSink _notificationSink = null!; + private readonly FailingReminderSessionPipeline _sessionPipeline = + new("persistence recovery failed"); public ReminderManagerActorTests(ITestOutputHelper output) : base(output: output) { } @@ -52,24 +55,24 @@ protected override void ConfigureAkka(AkkaConfigurationBuilder builder, IService { reminders.WithInMemoryStorage(); reminders.WithResolver(_ => sharedResolver); + reminders.WithSettings(new ReminderSettings + { + AckTimeout = TimeSpan.FromSeconds(2), + RetryBackoffBase = TimeSpan.FromMilliseconds(25), + MaxRetryBackoff = TimeSpan.FromMilliseconds(25), + MaxDeliveryAttempts = 10 + }); }); builder.StartActors((system, registry, _) => { - // Create a minimal SessionPipeline stub — manager needs it but - // we won't actually execute reminders in these tests. registry.Register(system.DeadLetters); - var pipeline = new SessionPipeline( - system, - new RequiredActor(ActorRegistry.For(system)), - new NetclawPaths(Path.Combine(Path.GetTempPath(), $"netclaw-test-{Guid.NewGuid():N}"))); - var defaults = new EffectivePolicyDefaults( DeploymentPosture.Team, TrustAudience.Team, ShellExecutionMode.Off, false); var reminderManager = system.ActorOf( Props.Create(() => new ReminderManagerActor( - pipeline, + _sessionPipeline, defaults, new SchedulingConfig(), _timeProvider, @@ -216,7 +219,26 @@ public async Task Status_query_for_unknown_reminder_returns_not_found() } [Fact] - public async Task Reconcile_deletes_zombie_oneshot_reminders() + public async Task Status_query_reads_durable_failure_count_after_store_reload() + { + var manager = await GetManagerAsync(); + var definition = CreateDefinition("durable-status", "Check status") with + { + ConsecutiveFailures = 3 + }; + _definitionStore.Save(definition); + + var status = await manager.Ask( + new GetReminderStatusQuery(definition.Id), + TimeSpan.FromSeconds(5), + TestContext.Current.CancellationToken); + + Assert.True(status.Found); + Assert.Equal(3, status.ConsecutiveFailures); + } + + [Fact] + public async Task Reconcile_retains_past_oneshot_without_durable_outcome() { var manager = await GetManagerAsync(); var now = TimeProvider.System.GetUtcNow(); @@ -252,6 +274,10 @@ public async Task Reconcile_deletes_zombie_oneshot_reminders() UpdatedAt = now.AddHours(-2) }; _definitionStore.Save(zombie); + var historyStore = new ReminderHistoryStore(new NetclawPaths(_basePath)); + await historyStore.AppendAsync( + zombie.Id, + new HistoryRecord(now.AddMinutes(-30), false, 100, "session-1", "recovery failed")); // Confirm it shows up as scheduled var healthBefore = await manager.Ask( @@ -261,11 +287,15 @@ public async Task Reconcile_deletes_zombie_oneshot_reminders() // Trigger reconciliation and wait for completion ack var reconcileResult = await manager.Ask( ReminderManagerActor.ReconcileReminders.Instance, TimeSpan.FromSeconds(30), TestContext.Current.CancellationToken); - Assert.Equal(1, reconcileResult.DeletedOneShots); + Assert.Equal(0, reconcileResult.SoftDeletedOneShots); - // Verify definition has been deleted from the store + // The missing occurrence status is ambiguous. Reconciliation must keep + // the definition and its history for operator review. See issue #1803. var afterReconcile = _definitionStore.Get(new ReminderId("zombie-oneshot")); - Assert.Null(afterReconcile); + Assert.NotNull(afterReconcile); + Assert.True(afterReconcile!.Enabled); + Assert.Null(afterReconcile.TerminalOutcome); + Assert.Single(await historyStore.ReadAsync(zombie.Id, 10)); } [Theory] @@ -562,7 +592,7 @@ public async Task Mode_B_reminder_dispatches_to_resolved_gateway_and_completes_o // gateway via ActorRegistry, and issued the Ask. The probe's // CommandAck reply + the subsequent _client.AckAsync path is // exercised end-to-end here too (execution actor didn't crash - // or report a failure up to the manager's _failureCounts); any + // or persist a failure on the reminder definition); any // failure in that tail would surface as a reminder execution // failure alert via the notification sink, which this test would // see on the next Ask to the manager if it happened. @@ -794,10 +824,7 @@ await AwaitAssertAsync(async () => TimeSpan.FromSeconds(3), TestContext.Current.CancellationToken); - Assert.Equal(0, health.FailedCount); - Assert.DoesNotContain(_notificationSink.Alerts, alert => - alert.Category == AlertType.ReminderExecutionFailed - && alert.Source == definition.Id.Value); + Assert.Equal(0, health.ActiveExecutions); }, duration: TimeSpan.FromSeconds(5), cancellationToken: TestContext.Current.CancellationToken); } finally @@ -1090,10 +1117,7 @@ await AwaitAssertAsync(async () => TimeSpan.FromSeconds(3), TestContext.Current.CancellationToken); - Assert.Equal(0, health.FailedCount); - Assert.DoesNotContain(_notificationSink.Alerts, alert => - alert.Category == AlertType.ReminderExecutionFailed - && alert.Source == definition.Id.Value); + Assert.Equal(0, health.ActiveExecutions); }, duration: TimeSpan.FromSeconds(5), cancellationToken: TestContext.Current.CancellationToken); } finally @@ -1103,7 +1127,87 @@ await AwaitAssertAsync(async () => } [Fact] - public async Task Second_fire_of_executing_reminder_is_skipped_as_duplicate() + public async Task Channel_failure_retries_and_fifth_failure_disables_reminder() + { + var manager = await GetManagerAsync(); + var definition = CreateDefinition("retry-poison", "Run the briefing") with + { + Schedule = new ReminderSchedule + { + Type = ReminderScheduleType.OneShot, + FireAt = _timeProvider.GetUtcNow().AddMilliseconds(100) + } + }; + + var saved = await manager.Ask( + new SaveReminderCommand( + definition, + Authorization: new ReminderAudienceAuthorizationContext(TrustAudience.Team, "test")), + TimeSpan.FromSeconds(5), + TestContext.Current.CancellationToken); + Assert.True(saved.Success, saved.ErrorMessage); + + await AwaitAssertAsync(() => + { + var stored = _definitionStore.Get(definition.Id); + Assert.NotNull(stored); + Assert.False(stored!.Enabled); + Assert.Equal(ReminderManagerActor.FailurePauseThreshold, stored.ConsecutiveFailures); + Assert.Equal(ReminderTerminalOutcome.Failed, stored.TerminalOutcome); + Assert.Equal(ReminderManagerActor.FailurePauseThreshold, _sessionPipeline.InvocationCount); + }, duration: TimeSpan.FromSeconds(5), cancellationToken: TestContext.Current.CancellationToken); + } + + [Fact] + public async Task Successful_retry_resets_failure_count_and_soft_deletes_oneshot() + { + var manager = await GetManagerAsync(); + var gatewayProbe = CreateTestProbe("retry-success-gateway"); + var gateway = Sys.ActorOf( + Props.Create(() => new AutoAckTrustedGateway(gatewayProbe.Ref)), + "auto-ack-retry-success-gateway"); + ActorRegistry.For(Sys).Register(gateway); + + var definition = CreateCurrentSessionDefinition("retry-success", deliveryRequired: false) with + { + ConsecutiveFailures = 3, + Schedule = new ReminderSchedule + { + Type = ReminderScheduleType.OneShot, + FireAt = _timeProvider.GetUtcNow().AddMilliseconds(100) + } + }; + + var saved = await manager.Ask( + new SaveReminderCommand( + definition, + Authorization: new ReminderAudienceAuthorizationContext(TrustAudience.Team, "test")), + TimeSpan.FromSeconds(5), + TestContext.Current.CancellationToken); + Assert.True(saved.Success, saved.ErrorMessage); + + await gatewayProbe.ExpectMsgAsync( + TimeSpan.FromSeconds(5), + cancellationToken: TestContext.Current.CancellationToken); + + await AwaitAssertAsync(async () => + { + var stored = _definitionStore.Get(definition.Id); + Assert.NotNull(stored); + Assert.False(stored!.Enabled); + Assert.Equal(0, stored.ConsecutiveFailures); + Assert.Equal(ReminderTerminalOutcome.Completed, stored.TerminalOutcome); + + var status = await manager.Ask( + new GetReminderStatusQuery(definition.Id), + TimeSpan.FromSeconds(3), + TestContext.Current.CancellationToken); + Assert.Equal("Delivered", status.Occurrence?.CompletionStatus); + }, duration: TimeSpan.FromSeconds(5), cancellationToken: TestContext.Current.CancellationToken); + } + + [Fact] + public async Task Second_fire_of_executing_reminder_waits_in_the_deferred_queue() { var manager = await GetManagerAsync(); @@ -1133,7 +1237,7 @@ await deliveryProbe.ExpectMsgAsync( TimeSpan.FromSeconds(5), cancellationToken: TestContext.Current.CancellationToken); await deliveryProbe.ExpectNoMsgAsync(TimeSpan.FromMilliseconds(300), TestContext.Current.CancellationToken); - // Health reflects one active execution — the duplicate was dropped, not queued. + // Health reflects one active execution. The second occurrence waits. var health = await manager.Ask( GetReminderHealthQuery.Instance, TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken); Assert.Equal(1, health.ActiveExecutions); @@ -1162,6 +1266,31 @@ public AutoAckTrustedGateway(IActorRef probe) } } + private sealed class FailingReminderSessionPipeline(string reason) : ISessionPipeline + { + private int _invocationCount; + + public int InvocationCount => Volatile.Read(ref _invocationCount); + + public Task CreateAsync( + SessionId sessionId, + SessionPipelineOptions options, + IMaterializer? materializer = null, + CancellationToken cancellationToken = default) + { + Interlocked.Increment(ref _invocationCount); + throw new InvalidOperationException(reason); + } + + public Task SendFeedbackAsync(IWithSessionId feedback, CancellationToken ct = default) => + Task.CompletedTask; + + public Task SendFeedbackAndWaitAsync( + IWithSessionId feedback, + CancellationToken ct = default) => + Task.FromResult(CommandAck.For(feedback.SessionId)); + } + /// /// Accepts messages and forwards them /// to a probe, but never replies to Sender. Keeps the execution actor's diff --git a/src/Netclaw.Actors/Channels/SessionPipelineHandle.cs b/src/Netclaw.Actors/Channels/SessionPipelineHandle.cs index dc588ebf3..96b7ff8b6 100644 --- a/src/Netclaw.Actors/Channels/SessionPipelineHandle.cs +++ b/src/Netclaw.Actors/Channels/SessionPipelineHandle.cs @@ -150,13 +150,34 @@ async Task ObserveTerminationAsync() /// /// Pipeline creation for fire-and-forget execution actors that use /// , offer input once, and complete. - /// Does not wire stream-terminated detection (the actor stops on TurnCompleted/ErrorOutput). + /// This compatibility overload keeps the prior stream lifecycle behavior. + /// + public Task> InitializeWithQueueAsync( + IActorContext context, + SessionId sessionId, + SessionPipelineOptions options, + Action onOutput, + CancellationToken cancellationToken = default) => + InitializeWithQueueAsync( + context, + sessionId, + options, + onOutput, + _ => { }, + cancellationToken); + + /// + /// Pipeline creation for fire-and-forget execution actors that use + /// , offer input once, and complete. + /// Reports stream termination so the owner can fail a session that ends + /// without a terminal output. /// public async Task> InitializeWithQueueAsync( IActorContext context, SessionId sessionId, SessionPipelineOptions options, Action onOutput, + Action onStreamTerminated, CancellationToken cancellationToken = default) { _log.Info("Initializing {0} execution pipeline", _materializerNamePrefix); @@ -177,17 +198,46 @@ public async Task> InitializeWithQueueAsy // sessions don't leak the fault on teardown. StreamTaskObservation.ObserveSilently(inputQueue.WatchCompletionAsync()); - _outputCompletion = materialized.Output + var outputTerminated = materialized.Output .WatchTermination((_, done) => done) .ToMaterialized( Sink.ForEach(onOutput).ObservingFault(), Keep.Left) .Run(materializer); + _outputCompletion = outputTerminated; + + _ = ObserveTerminationAsync(); + _session = materialized; _log.Info("{0} execution pipeline initialized", _materializerNamePrefix); return inputQueue; + + async Task ObserveTerminationAsync() + { + Exception? failure = null; + try + { + await outputTerminated.ConfigureAwait(false); + } + catch (Exception ex) + { + failure = ex; + } + + try + { + onStreamTerminated(failure); + } + catch (Exception callbackException) + { + _log.Error( + callbackException, + "{0} onStreamTerminated callback threw", + _materializerNamePrefix); + } + } } /// diff --git a/src/Netclaw.Actors/Hosting/NetclawAkkaHostingExtensions.cs b/src/Netclaw.Actors/Hosting/NetclawAkkaHostingExtensions.cs index 449b2814e..ca4e51ce5 100644 --- a/src/Netclaw.Actors/Hosting/NetclawAkkaHostingExtensions.cs +++ b/src/Netclaw.Actors/Hosting/NetclawAkkaHostingExtensions.cs @@ -22,6 +22,8 @@ namespace Netclaw.Actors.Hosting; public static class NetclawAkkaHostingExtensions { + internal static readonly TimeSpan ReminderAckTimeout = TimeSpan.FromMinutes(70); + public sealed record ReminderStorageOptions { public string? SqliteConnectionString { get; init; } @@ -68,11 +70,8 @@ public static AkkaConfigurationBuilder WithModelCapabilityCache( /// /// Registers the reminder manager as a singleton actor and wires /// the local Akka.Reminders scheduler to deliver payloads to it. - /// Uses Akka.Reminders' built-in default settings throughout — no - /// configuration surface exposed. If operators ever need to tune - /// AckTimeout, MaxRetryBackoff, or - /// MaxDeliveryAttempts, a configuration knob can be added at - /// that point. Right now: YAGNI. + /// Uses a 70-minute acknowledgement lease for one-hour LLM attempts. + /// Other Akka.Reminders settings use their library defaults. /// public static AkkaConfigurationBuilder WithReminderManager( this AkkaConfigurationBuilder builder, @@ -86,6 +85,11 @@ public static AkkaConfigurationBuilder WithReminderManager( return builder .WithLocalReminders(reminders => { + reminders.WithSettings(new ReminderSettings + { + AckTimeout = ReminderAckTimeout + }); + if (!string.IsNullOrWhiteSpace(storageOptions?.SqliteConnectionString)) { reminders.WithStorage(system => diff --git a/src/Netclaw.Actors/Reminders/ActiveExecutionTracker.cs b/src/Netclaw.Actors/Reminders/ActiveExecutionTracker.cs index 3ac72ca22..78642e407 100644 --- a/src/Netclaw.Actors/Reminders/ActiveExecutionTracker.cs +++ b/src/Netclaw.Actors/Reminders/ActiveExecutionTracker.cs @@ -6,20 +6,43 @@ namespace Netclaw.Actors.Reminders; +using Akka.Reminders; + /// /// Tracks in-flight reminder executions. /// Enforces the invariant that only one execution of a given reminder runs at a time. /// internal sealed class ActiveExecutionTracker { - private readonly HashSet _executing = []; + private readonly Dictionary _executing = []; public int Count => _executing.Count; - public bool IsExecuting(ReminderId reminderId) => _executing.Contains(reminderId); + public bool IsExecuting(ReminderId reminderId) => _executing.ContainsKey(reminderId); + + public void Add( + ReminderId reminderId, + Guid executionId, + ReminderEnvelope envelope) => + _executing.Add(reminderId, new ActiveReminderExecution(executionId, envelope)); - public void Add(ReminderId reminderId) => _executing.Add(reminderId); + public bool TryRemove( + ReminderId reminderId, + Guid executionId, + out ActiveReminderExecution execution) + { + if (_executing.TryGetValue(reminderId, out execution!) + && execution.ExecutionId == executionId) + { + _executing.Remove(reminderId); + return true; + } - /// true if the reminder was tracked and has been removed. - public bool Remove(ReminderId reminderId) => _executing.Remove(reminderId); + execution = null!; + return false; + } } + +internal sealed record ActiveReminderExecution( + Guid ExecutionId, + ReminderEnvelope Envelope); diff --git a/src/Netclaw.Actors/Reminders/ReminderExecutionActor.cs b/src/Netclaw.Actors/Reminders/ReminderExecutionActor.cs index 047fbc92f..e38987d17 100644 --- a/src/Netclaw.Actors/Reminders/ReminderExecutionActor.cs +++ b/src/Netclaw.Actors/Reminders/ReminderExecutionActor.cs @@ -51,6 +51,12 @@ internal sealed class ReminderExecutionActor : ReceiveActor /// internal static TimeSpan ExecutionStallTimeout = TimeSpan.FromMinutes(20); + /// + /// Absolute limit for one execution attempt. This limit applies while output remains active. + /// The Akka.Reminders acknowledgement lease exceeds this limit. + /// + internal static TimeSpan ExecutionAttemptTimeout = TimeSpan.FromHours(1); + private readonly Guid _executionId; private readonly ReminderDefinition _definition; private readonly ReminderHistoryStore _historyStore; @@ -68,6 +74,8 @@ internal sealed class ReminderExecutionActor : ReceiveActor private bool _awaitingDeliveryResult; private ReminderId? _expectedReminderDeliveryKey; private ICancelable? _deliveryTimeoutCancelable; + private ICancelable? _executionTimeoutCancelable; + private bool _settlementStarted; private bool RoutesBackToOriginSession => _definition.Delivery.Kind == DeliveryKind.CurrentSession; @@ -114,6 +122,8 @@ public ReminderExecutionActor( Receive(_ => { }); Receive(HandleDeliveryResult); Receive(HandleDeliveryBackstopTimeout); + Receive(_ => HandleExecutionAttemptTimeout()); + Receive(HandleOutputStreamTerminated); Receive(_ => HandleExecutionStall()); } @@ -122,12 +132,19 @@ protected override void PreStart() _log.Info( $"ReminderExecution Dispatched: execution_id={_executionId} reminder_id={_definition.Id} title={_definition.Title} schedule_type={_definition.Schedule.Type} dispatched_at={_dispatchedAt} delivery_kind={_definition.Delivery.Kind}"); - if (RoutesBackToOriginSession) + if (_envelope is not null) { - _reminderClient = ReminderClientExtension.Get(Context.System) - .CreateClient(new ReminderEntity(ReminderManagerActor.ShardRegionName, ReminderManagerActor.EntityId)); + var extension = ReminderClientExtension.Get(Context.System); + _reminderClient = extension.CreateClient( + new ReminderEntity(ReminderManagerActor.ShardRegionName, ReminderManagerActor.EntityId)); } + _executionTimeoutCancelable = Context.System.Scheduler.ScheduleTellOnceCancelable( + ExecutionAttemptTimeout, + Self, + ExecutionAttemptTimeoutReached.Instance, + Self); + Self.Tell(new ExecutionStarted()); RunTask(RoutesBackToOriginSession ? InitializeCurrentSessionAsync : InitializeAsync); } @@ -138,7 +155,7 @@ private async Task InitializeAsync() { var sessionId = !string.IsNullOrWhiteSpace(_definition.Delivery.SessionId) ? new SessionId(_definition.Delivery.SessionId) - : new SessionId($"reminder/{_definition.Id}/{_timeProvider.GetUtcNow().ToUnixTimeMilliseconds()}"); + : new SessionId($"reminder/{_definition.Id}/{(_envelope?.DueTimeUtc ?? _dispatchedAt).ToUnixTimeMilliseconds()}"); _sessionIdValue = sessionId.Value; var audience = _definition.Audience; @@ -156,7 +173,8 @@ private async Task InitializeAsync() ChannelType = Channels.ChannelType.Reminder, Filter = OutputFilter.TextStreaming | OutputFilter.ToolCalls }, - output => self.Tell(new ExecutionOutput(output))); + output => self.Tell(new ExecutionOutput(output)), + failure => self.Tell(new OutputStreamTerminated(failure))); var prompt = BuildPrompt(_definition); @@ -291,7 +309,6 @@ private async Task InitializeCurrentSessionAsync() break; } - await TryAckEnvelopeAsync(); ReportAndStop(true); break; @@ -366,22 +383,7 @@ private void HandleDeliveryResult(ReminderDeliveryResult result) _log.Info( "reminder_delivery_observed execution_id={ExecutionId} reminder_id={ReminderId} key={ReminderDeliveryKey} channel={ChannelType}", _executionId, _definition.Id, result.ReminderDeliveryKey, result.ChannelType); - RunTask(async () => - { - try - { - await TryAckEnvelopeAsync(); - ReportAndStop(true); - } - catch (Exception ex) - { - // Never let an ack fault escalate to a supervisor restart: - // that would re-run PreStart and re-post the reminder turn - // in a loop. Report failure (no ack) and stop instead. - LogFullException(ex, "ReminderExecution AckFailed"); - ReportAndStop(false, ex.Message); - } - }); + ReportAndStop(true); } else { @@ -408,21 +410,42 @@ private void HandleDeliveryBackstopTimeout(DeliveryBackstopTimeout msg) ReportAndStop(false, $"delivery not observed within {DeliveryObservedTimeout}"); } - private async Task TryAckEnvelopeAsync() + private async Task<(bool Success, string? ErrorMessage, bool OccurrenceTerminal)> SettleOccurrenceAsync( + bool executionSucceeded, + string? errorMessage) { - // No envelope to ack when the reminder was re-run from the deferred - // queue: that path already acked-and-dropped the envelope eagerly - // (ReminderManagerActor concurrency gate). Acking null would throw. if (_envelope is null) - return; + return (executionSucceeded, errorMessage, false); - var ackResponse = await _reminderClient!.AckAsync(_envelope); - if (ackResponse.ResponseCode != ReminderAckResponseCode.Success) + if (executionSucceeded) { + var ackResponse = await _reminderClient!.AckAsync(_envelope); + if (ackResponse.ResponseCode == ReminderAckResponseCode.Success) + return (true, null, false); + + var ackError = ackResponse.Message ?? $"Reminder acknowledgement returned {ackResponse.ResponseCode}."; _log.Warning( "reminder_ack_non_success execution_id={ExecutionId} reminder_id={ReminderId} response={ResponseCode} message={Message}", _executionId, _definition.Id, ackResponse.ResponseCode, ackResponse.Message); + return (false, ackError, false); + } + + var reason = string.IsNullOrWhiteSpace(errorMessage) + ? "Reminder execution failed." + : errorMessage; + var nackResponse = await _reminderClient!.NackAsync(_envelope, reason); + var terminal = nackResponse.ResponseCode is ReminderNackResponseCode.Failed + or ReminderNackResponseCode.Expired; + + if (nackResponse.ResponseCode is ReminderNackResponseCode.Error + or ReminderNackResponseCode.NotFound) + { + _log.Warning( + "reminder_nack_non_success execution_id={ExecutionId} reminder_id={ReminderId} response={ResponseCode} message={Message}", + _executionId, _definition.Id, nackResponse.ResponseCode, nackResponse.Message); } + + return (false, reason, terminal); } private IActorRef? ResolveGatewayFor(ChannelType originChannelType) @@ -575,15 +598,61 @@ private void HandleExecutionStall() ReportAndStop(false, $"Reminder execution stalled: no session output for {ExecutionStallTimeout}."); } + private void HandleExecutionAttemptTimeout() + { + if (_completed || _settlementStarted) + return; + + _log.Warning( + "ReminderExecution reached its absolute limit: execution_id={0} reminder_id={1} title={2} timeout={3}.", + _executionId, _definition.Id, _definition.Title, ExecutionAttemptTimeout); + ReportAndStop(false, $"Reminder execution exceeded {ExecutionAttemptTimeout}."); + } + + private void HandleOutputStreamTerminated(OutputStreamTerminated terminated) + { + if (_completed || _settlementStarted) + return; + + var reason = terminated.Failure?.Message ?? "Session output ended without a terminal result."; + ReportAndStop(false, reason); + } + private void ReportAndStop(bool success, string? errorMessage = null) { - if (_completed) + if (_completed || _settlementStarted) return; - _completed = true; + _settlementStarted = true; - // Disarm the Mode A stall backstop so it cannot fire during the drain. Context.SetReceiveTimeout(null); + _executionTimeoutCancelable?.Cancel(); + _executionTimeoutCancelable = null; + + RunTask(async () => + { + try + { + var settlement = await SettleOccurrenceAsync(success, errorMessage); + CompleteExecution( + settlement.Success, + settlement.ErrorMessage, + settlement.OccurrenceTerminal); + } + catch (Exception ex) + { + LogFullException(ex, "ReminderExecution SettlementFailed"); + CompleteExecution(false, ex.Message, occurrenceTerminal: false); + } + + await _handle.DrainAsync(); + Context.Stop(Self); + }); + } + + private void CompleteExecution(bool success, string? errorMessage, bool occurrenceTerminal) + { + _completed = true; var durationMs = (long)(_timeProvider.GetUtcNow() - _dispatchedAt).TotalMilliseconds; _pendingHistory = new HistoryRecord( @@ -603,21 +672,16 @@ private void ReportAndStop(bool success, string? errorMessage = null) _executionId, _definition.Id, success, - errorMessage)); - - // Drain stream stages before stopping so they complete gracefully - // rather than being abruptly terminated as actor children. - RunTask(async () => - { - await _handle.DrainAsync(); - Context.Stop(Self); - }); + errorMessage, + occurrenceTerminal)); } protected override void PostStop() { _deliveryTimeoutCancelable?.Cancel(); _deliveryTimeoutCancelable = null; + _executionTimeoutCancelable?.Cancel(); + _executionTimeoutCancelable = null; if (_pendingHistory is not null) { @@ -651,6 +715,12 @@ private void LogFullException(Exception ex, string phase) private sealed record ExecutionStarted : INoSerializationVerificationNeeded; private sealed record ExecutionOutput(SessionOutput Output) : INoSerializationVerificationNeeded; + private sealed record OutputStreamTerminated(Exception? Failure) : INoSerializationVerificationNeeded; + + private sealed record ExecutionAttemptTimeoutReached : INoSerializationVerificationNeeded + { + public static readonly ExecutionAttemptTimeoutReached Instance = new(); + } /// /// Self-scheduled backstop fired when no diff --git a/src/Netclaw.Actors/Reminders/ReminderManagerActor.cs b/src/Netclaw.Actors/Reminders/ReminderManagerActor.cs index 599767cd0..a387f4547 100644 --- a/src/Netclaw.Actors/Reminders/ReminderManagerActor.cs +++ b/src/Netclaw.Actors/Reminders/ReminderManagerActor.cs @@ -53,8 +53,7 @@ public sealed partial class ReminderManagerActor : ReceiveActor private IReminderClient? _client; private readonly ActiveExecutionTracker _activeExecutions = new(); - private readonly Queue _deferredQueue = new(); - private readonly Dictionary _failureCounts = []; + private readonly Queue _deferredQueue = new(); private readonly Dictionary _skipCounts = []; public ReminderManagerActor( @@ -87,6 +86,7 @@ public ReminderManagerActor( ReceiveAsync>(HandleReminderFiredAsync); ReceiveAsync(HandleExecutionCompletedAsync); + ReceiveAsync(HandleExecutionTerminatedAsync); ReceiveAsync(_ => HandleReconcileAsync()); Receive(_ => HandleGetHealth()); @@ -115,6 +115,9 @@ protected override void PreStart() Self.Tell(ReconcileReminders.Instance); } + protected override SupervisorStrategy SupervisorStrategy() => + new OneForOneStrategy(_ => Directive.Stop); + private void EmitDroppedInvalidDefinitionAlerts() { var dropped = _definitionStore.ConsumeDroppedInvalidDefinitions(); @@ -428,7 +431,6 @@ private async Task DisableReminderInternalAsync(ReminderI _definitionStore.Save(definition); await CancelScheduleOnlyAsync(id); - _failureCounts.Remove(id); _skipCounts.Remove(id); RemoveFromDeferredQueue(id); @@ -437,15 +439,13 @@ private async Task DisableReminderInternalAsync(ReminderI } /// - /// Permanently removes a reminder definition, its schedule, history, and any - /// in-memory tracking state. Used during startup reconciliation to clean up - /// stale one-shot reminders whose fire time has passed. + /// Permanently removes a reminder definition, its schedule, history, and process state. + /// Only an explicit delete command uses this path. /// private async Task DeleteReminderInternalAsync(ReminderId id) { _definitionStore.Delete(id); await CancelScheduleOnlyAsync(id); - _failureCounts.Remove(id); _skipCounts.Remove(id); RemoveFromDeferredQueue(id); @@ -468,6 +468,8 @@ private async Task EnableReminderInternalAsync(ReminderId definition = definition with { Enabled = true, + ConsecutiveFailures = 0, + TerminalOutcome = null, UpdatedAtMs = _timeProvider.GetUtcNow().ToUnixTimeMilliseconds() }; @@ -591,7 +593,7 @@ private async Task HandleReminderFiredAsync(ReminderEnvelope en if (_activeExecutions.IsExecuting(reminderId)) { RecordSkippedDuplicate(reminderId, definition.Title, "scheduled"); - await _client!.AckAsync(envelope); + EnqueueDeferredOccurrence(envelope); return; } @@ -605,40 +607,34 @@ private async Task HandleReminderFiredAsync(ReminderEnvelope en } } - var isCurrentSessionDelivery = definition.Delivery.Kind == DeliveryKind.CurrentSession; - if (_activeExecutions.Count >= MaxConcurrentExecutions) { _log.Info("Concurrency limit reached ({0}), deferring reminder '{1}'", MaxConcurrentExecutions, reminderId.Value); - _deferredQueue.Enqueue(reminderId); - // Ack even Mode B envelopes on the deferred path — the - // concurrency gate fires before we can dispatch to the - // gateway, so holding the envelope open would starve - // Akka.Reminders' retry budget on nothing. - await _client!.AckAsync(envelope); + EnqueueDeferredOccurrence(envelope); return; } - if (isCurrentSessionDelivery) - { - // CurrentSession: execution actor holds the envelope open and acks - // itself once the target session has confirmed receipt. - StartExecution(definition, envelope); - } - else + // Issue #1803: every delivery mode retains its envelope until the + // execution actor confirms success or reports a known failure. + StartExecution(definition, envelope); + } + + private void EnqueueDeferredOccurrence(ReminderEnvelope envelope) + { + if (_deferredQueue.Any(item => + item.Envelope.Key == envelope.Key + && item.Envelope.DueTimeUtc == envelope.DueTimeUtc)) { - // Channel/None: ack envelope eagerly, execution tracks its own success - StartExecution(definition); - await _client!.AckAsync(envelope); + return; } + + _deferredQueue.Enqueue(new DeferredReminderOccurrence(envelope.Message.Id, envelope)); } /// - /// Records a skipped fire (a fire that arrived while a prior execution of the - /// same reminder was still running). Counted in-memory since daemon start and - /// surfaced by netclaw reminder status so the silent-skip pattern from - /// #1492/#1494 (49 skips in a day, unnoticed) is now visible to operators. + /// Records an occurrence that waits while the same reminder runs. + /// The status command exposes this in-memory overlap count. /// private void RecordSkippedDuplicate(ReminderId reminderId, string title, string source) { @@ -674,21 +670,67 @@ private void PostFailureNoticeToChannel(ReminderDefinition? definition, string t private async Task HandleExecutionCompletedAsync(ReminderExecutionCompleted completed) { - if (!_activeExecutions.Remove(completed.Id)) + if (!_activeExecutions.TryRemove(completed.Id, completed.ExecutionId, out _)) return; + await ApplyExecutionResultAsync(completed); + } + + private async Task HandleExecutionTerminatedAsync(ReminderExecutionTerminated terminated) + { + if (!_activeExecutions.TryRemove(terminated.Id, terminated.ExecutionId, out var execution)) + return; + + const string reason = "Reminder execution actor terminated unexpectedly."; + var nack = await _client!.NackAsync(execution.Envelope, reason); + var terminal = nack.ResponseCode is ReminderNackResponseCode.Failed + or ReminderNackResponseCode.Expired; + + await ApplyExecutionResultAsync(new ReminderExecutionCompleted( + terminated.ExecutionId, + terminated.Id, + Success: false, + ErrorMessage: reason, + OccurrenceTerminal: terminal)); + } + + private async Task ApplyExecutionResultAsync(ReminderExecutionCompleted completed) + { var definition = _definitionStore.Get(completed.Id); var title = definition?.Title ?? completed.Id.Value; if (completed.Success) { - _failureCounts.Remove(completed.Id); + if (definition is not null) + { + definition = definition with + { + ConsecutiveFailures = 0, + Enabled = definition.Schedule.Type is ReminderScheduleType.OneShot + ? false + : definition.Enabled, + TerminalOutcome = definition.Schedule.Type is ReminderScheduleType.OneShot + ? ReminderTerminalOutcome.Completed + : null, + UpdatedAtMs = _timeProvider.GetUtcNow().ToUnixTimeMilliseconds() + }; + _definitionStore.Save(definition); + } + _log.Info("Reminder '{0}' execution completed successfully", completed.Id.Value); } else { - var count = _failureCounts.GetValueOrDefault(completed.Id) + 1; - _failureCounts[completed.Id] = count; + var count = (definition?.ConsecutiveFailures ?? 0) + 1; + if (definition is not null) + { + definition = definition with + { + ConsecutiveFailures = count, + UpdatedAtMs = _timeProvider.GetUtcNow().ToUnixTimeMilliseconds() + }; + _definitionStore.Save(definition); + } _log.Warning("Reminder '{0}' execution failed ({1}/{2}): {3}", completed.Id.Value, @@ -723,11 +765,14 @@ private async Task HandleExecutionCompletedAsync(ReminderExecutionCompleted comp $"Reminder \"{title}\" failed: {completed.ErrorMessage ?? "unknown error"}"); } - if (count >= FailurePauseThreshold) + if (count >= FailurePauseThreshold || completed.OccurrenceTerminal) { - _log.Warning("Reminder '{0}' hit failure threshold ({1}), disabling", + var disableReason = count >= FailurePauseThreshold + ? $"failure threshold ({FailurePauseThreshold})" + : "the occurrence retry budget"; + _log.Warning("Reminder '{0}' hit {1}, disabling", completed.Id.Value, - FailurePauseThreshold); + disableReason); _notificationSink.Emit(OperationalAlert.Create( _timeProvider, @@ -748,18 +793,20 @@ private async Task HandleExecutionCompletedAsync(ReminderExecutionCompleted comp $"Reminder \"{title}\" was automatically disabled after {count} consecutive failures. " + $"Last error: {completed.ErrorMessage ?? "unknown error"}"); - await DisableReminderInternalAsync(completed.Id); - _failureCounts.Remove(completed.Id); - } - } + if (definition is not null) + { + definition = definition with + { + Enabled = false, + TerminalOutcome = ReminderTerminalOutcome.Failed, + UpdatedAtMs = _timeProvider.GetUtcNow().ToUnixTimeMilliseconds() + }; + _definitionStore.Save(definition); + } - // One-shot reminders cannot fire again — soft-delete by disabling. - // The definition stays on disk so history remains queryable. - // Startup reconciliation will hard-delete stale disabled one-shots. - if (definition is { Schedule.Type: ReminderScheduleType.OneShot }) - { - _log.Info("One-shot reminder '{0}' completed, disabling (soft-delete)", completed.Id.Value); - await DisableReminderInternalAsync(completed.Id); + await CancelScheduleOnlyAsync(completed.Id); + RemoveFromDeferredQueue(completed.Id); + } } await ProcessDeferredQueueAsync(); @@ -790,23 +837,54 @@ private async Task HandleReconcileAsync() if (scheduled.ContainsKey(definition.Id.Value)) continue; + if (definition.Schedule.Type == ReminderScheduleType.OneShot + && definition.Schedule.FireAt <= _timeProvider.GetUtcNow()) + { + continue; + } + var result = await ScheduleDefinitionAsync(definition, rescheduleFromNow: true); if (result.IsSuccess) restoredSchedules++; } - // Delete stale one-shots: definitions with fire time in the past - // and no active Akka.Reminders schedule (already fired, never cleaned up). - // Includes both enabled zombies and already-disabled leftovers. + // Issue #1803: a past due time and the absence of a schedule do not prove success. var now = _timeProvider.GetUtcNow(); - var deletedOneShots = 0; + var softDeletedOneShots = 0; foreach (var definition in definitions.Where(d => + d.Enabled && d.Schedule.Type == ReminderScheduleType.OneShot && - d.Schedule.FireAt <= now && - !scheduled.ContainsKey(d.Id.Value))) + d.Schedule.FireAt <= now)) { - await DeleteReminderInternalAsync(definition.Id); - deletedOneShots++; + var occurrence = await GetOccurrenceStatusAsync(definition); + if (occurrence is null) + { + _log.Warning( + "Past one-shot reminder '{0}' has no durable occurrence status. The definition remains enabled for operator review.", + definition.Id.Value); + continue; + } + + var outcome = occurrence.CompletionStatus switch + { + Akka.Reminders.Storage.ReminderCompletionStatus.Delivered => ReminderTerminalOutcome.Completed, + Akka.Reminders.Storage.ReminderCompletionStatus.Failed => ReminderTerminalOutcome.Failed, + Akka.Reminders.Storage.ReminderCompletionStatus.Expired => ReminderTerminalOutcome.Failed, + Akka.Reminders.Storage.ReminderCompletionStatus.Cancelled => ReminderTerminalOutcome.Failed, + _ => (ReminderTerminalOutcome?)null + }; + + if (outcome is null) + continue; + + var terminalDefinition = definition with + { + Enabled = false, + TerminalOutcome = outcome, + UpdatedAtMs = now.ToUnixTimeMilliseconds() + }; + _definitionStore.Save(terminalDefinition); + softDeletedOneShots++; } // Disable expired recurring reminders that haven't fired since expiration. @@ -820,33 +898,34 @@ d.Schedule.Type is not ReminderScheduleType.OneShot && disabledExpired++; } - if (cancelledOrphans > 0 || restoredSchedules > 0 || deletedOneShots > 0 || disabledExpired > 0) + if (cancelledOrphans > 0 || restoredSchedules > 0 || softDeletedOneShots > 0 || disabledExpired > 0) { - _log.Info("Reminder reconcile complete: cancelled_orphans={0}, restored={1}, deleted_oneshots={2}, disabled_expired={3}", + _log.Info("Reminder reconcile complete: cancelled_orphans={0}, restored={1}, soft_deleted_oneshots={2}, disabled_expired={3}", cancelledOrphans, restoredSchedules, - deletedOneShots, + softDeletedOneShots, disabledExpired); } // Only ack external callers — skip Self.Tell from PreStart if (!sender.Equals(Self)) - sender.Tell(new ReconcileCompleted(cancelledOrphans, restoredSchedules, deletedOneShots, disabledExpired)); + sender.Tell(new ReconcileCompleted(cancelledOrphans, restoredSchedules, softDeletedOneShots, disabledExpired)); } catch (Exception ex) { _log.Error(ex, "Reminder reconcile failed"); - // Reply with a zero-result ack so external Ask callers don't hang until timeout if (!sender.Equals(Self)) - sender.Tell(new ReconcileCompleted(0, 0, 0)); + sender.Tell(new Status.Failure(ex)); } } - private void StartExecution(ReminderDefinition definition, ReminderEnvelope? envelope = null) + private void StartExecution( + ReminderDefinition definition, + ReminderEnvelope envelope) { var executionId = Guid.NewGuid(); - _activeExecutions.Add(definition.Id); + _activeExecutions.Add(definition.Id, executionId, envelope); var actorName = $"exec-{SanitizeActorName(definition.Id.Value)}-{_timeProvider.GetUtcNow().ToUnixTimeMilliseconds()}"; var executionActor = Context.ActorOf( @@ -858,24 +937,34 @@ private void StartExecution(ReminderDefinition definition, ReminderEnvelope 0 && _activeExecutions.Count < MaxConcurrentExecutions) + var candidates = _deferredQueue.Count; + while (_deferredQueue.Count > 0 + && _activeExecutions.Count < MaxConcurrentExecutions + && candidates-- > 0) { - var nextId = _deferredQueue.Dequeue(); + var deferred = _deferredQueue.Dequeue(); + var nextId = deferred.Id; var definition = _definitionStore.Get(nextId); if (definition is null || !definition.Enabled) + { + await _client!.AckAsync(deferred.Envelope); continue; + } if (_activeExecutions.IsExecuting(nextId)) { - RecordSkippedDuplicate(nextId, definition.Title, "deferred_queue"); + EnqueueDeferredOccurrence(deferred.Envelope); continue; } @@ -889,7 +978,7 @@ private async Task ProcessDeferredQueueAsync() continue; } - StartExecution(definition); + StartExecution(definition, deferred.Envelope); } } @@ -898,11 +987,11 @@ private void RemoveFromDeferredQueue(ReminderId id) if (_deferredQueue.Count == 0) return; - var keep = new Queue(); + var keep = new Queue(); while (_deferredQueue.Count > 0) { var item = _deferredQueue.Dequeue(); - if (item != id) + if (item.Id != id) keep.Enqueue(item); } @@ -1036,7 +1125,9 @@ private async Task CancelScheduleOnlyAsync(ReminderId id) Enabled: d.Enabled, AgentDefinitionId: d.AgentDefinitionId, Audience: d.Audience, - ExpiresAt: d.ExpiresAt); + ExpiresAt: d.ExpiresAt, + ConsecutiveFailures: d.ConsecutiveFailures, + TerminalOutcome: d.TerminalOutcome); private static string SanitizeActorName(string raw) { @@ -1054,7 +1145,7 @@ private void HandleGetHealth() Sender.Tell(new ReminderHealthResponse( scheduledCount, _activeExecutions.Count, - _failureCounts.Count)); + _definitionStore.List().Count(d => d.ConsecutiveFailures > 0))); } private async Task HandleGetStatusAsync(GetReminderStatusQuery query) @@ -1068,6 +1159,7 @@ private async Task HandleGetStatusAsync(GetReminderStatusQuery query) replyTo.Tell(new ReminderStatusResponse( query.Id, Found: false, Enabled: false, Executing: false, NextFire: null, ConsecutiveFailures: 0, SkippedDuplicates: 0, + TerminalOutcome: null, Occurrence: null, RecentHistory: [])); return; } @@ -1077,7 +1169,20 @@ private async Task HandleGetStatusAsync(GetReminderStatusQuery query) // Neither touches actor state until both complete. var schedulesTask = ListScheduledRemindersAsync(); var historyTask = _historyStore.ReadAsync(query.Id, RecentHistoryCount); - await Task.WhenAll(schedulesTask, historyTask); + var occurrenceTask = GetOccurrenceStatusAsync(definition); + await Task.WhenAll(schedulesTask, historyTask, occurrenceTask); + + var occurrence = occurrenceTask.Result is { } status + ? new ReminderOccurrenceInfo( + status.DueTimeUtc, + status.NextAttemptAtUtc, + status.AttemptCount, + status.LastFailureReason, + status.CompletionStatus.ToString(), + status.DeliveryDeadlineUtc, + status.AckDeadlineUtc, + status.CompletedAtUtc) + : null; replyTo.Tell(new ReminderStatusResponse( query.Id, @@ -1085,8 +1190,10 @@ private async Task HandleGetStatusAsync(GetReminderStatusQuery query) Enabled: definition.Enabled, Executing: _activeExecutions.IsExecuting(query.Id), NextFire: schedulesTask.Result.GetValueOrDefault(query.Id.Value), - ConsecutiveFailures: _failureCounts.GetValueOrDefault(query.Id), + ConsecutiveFailures: definition.ConsecutiveFailures, SkippedDuplicates: _skipCounts.GetValueOrDefault(query.Id), + TerminalOutcome: definition.TerminalOutcome, + Occurrence: occurrence, RecentHistory: historyTask.Result)); } catch (Exception ex) @@ -1101,12 +1208,40 @@ private async Task HandleGetStatusAsync(GetReminderStatusQuery query) } } + private async Task GetOccurrenceStatusAsync(ReminderDefinition definition) + { + if (_client is null + || definition.Schedule.Type is not ReminderScheduleType.OneShot + || definition.Schedule.FireAt is not { } dueTimeUtc) + { + return null; + } + + var response = await _client.GetOccurrenceStatusAsync( + new ReminderKey(definition.Id.Value), + dueTimeUtc); + + return response.ResponseCode switch + { + ReminderOccurrenceStatusResponseCode.Success => response.Status, + ReminderOccurrenceStatusResponseCode.NotFound => null, + ReminderOccurrenceStatusResponseCode.Error => throw new InvalidOperationException( + response.Message ?? "The reminder occurrence status query failed."), + _ => throw new InvalidOperationException( + $"Unexpected occurrence status response: {response.ResponseCode}.") + }; + } + private sealed record ScheduleAttempt(bool IsSuccess, DateTimeOffset? NextFire, string? ErrorMessage) : INoSerializationVerificationNeeded { public static ScheduleAttempt Ok(DateTimeOffset? nextFire) => new(true, nextFire, null); public static ScheduleAttempt Fail(string message) => new(false, null, message); } + private sealed record DeferredReminderOccurrence( + ReminderId Id, + ReminderEnvelope Envelope) : INoSerializationVerificationNeeded; + private sealed record ReminderAudienceAuthorizationResult(bool IsSuccess, TrustAudience? EffectiveAudience, string? ErrorMessage) : INoSerializationVerificationNeeded { public static ReminderAudienceAuthorizationResult Success(TrustAudience effectiveAudience) @@ -1134,5 +1269,9 @@ internal sealed record ReconcileReminders : INoSerializationVerificationNeeded /// Ack sent back to callers so they can /// synchronize on reconcile completion instead of polling. /// - internal sealed record ReconcileCompleted(int CancelledOrphans, int RestoredSchedules, int DeletedOneShots, int DisabledExpired = 0) : INoSerializationVerificationNeeded; + internal sealed record ReconcileCompleted( + int CancelledOrphans, + int RestoredSchedules, + int SoftDeletedOneShots, + int DisabledExpired = 0) : INoSerializationVerificationNeeded; } diff --git a/src/Netclaw.Actors/Reminders/ReminderProtocol.cs b/src/Netclaw.Actors/Reminders/ReminderProtocol.cs index 15c1da6d0..9a4179a32 100644 --- a/src/Netclaw.Actors/Reminders/ReminderProtocol.cs +++ b/src/Netclaw.Actors/Reminders/ReminderProtocol.cs @@ -198,6 +198,18 @@ public sealed record ReminderDefinition public bool Enabled { get; set; } = true; + /// + /// Number of consecutive failed execution attempts for this reminder. + /// A successful attempt resets this value. + /// + public int ConsecutiveFailures { get; set; } + + /// + /// Terminal result for a retained one-shot reminder. + /// Null means that the reminder can still run. + /// + public ReminderTerminalOutcome? TerminalOutcome { get; set; } + /// /// Deferred shadow field for selecting specialized agent behavior. /// Tracked by issue #147. @@ -254,6 +266,12 @@ public DateTimeOffset? ExpiresAt } } +public enum ReminderTerminalOutcome +{ + Completed, + Failed +} + /// /// Message persisted inside Akka.Reminders. Intentionally lightweight: pointer to disk definition. /// @@ -413,10 +431,8 @@ public sealed record GetReminderStatusQuery(ReminderId Id) : IReminderQuery, INo /// /// Response to : per-reminder health for an /// operator — whether the reminder exists/is enabled, whether an execution is in -/// flight right now, when it next fires, the consecutive-failure and -/// skipped-duplicate counts (in-memory since daemon start), and recent run -/// history. Lets netclaw reminder status answer "is this reminder healthy -/// or is it silently failing/skipping?" — the gap that hid #1492. +/// flight right now, when it next fires, the durable failure count, the +/// in-memory overlap count, and recent run history. /// public sealed record ReminderStatusResponse( ReminderId Id, @@ -426,6 +442,8 @@ public sealed record ReminderStatusResponse( DateTimeOffset? NextFire, int ConsecutiveFailures, int SkippedDuplicates, + ReminderTerminalOutcome? TerminalOutcome, + ReminderOccurrenceInfo? Occurrence, IReadOnlyList RecentHistory) : IReminderResponse, INoSerializationVerificationNeeded; } @@ -442,7 +460,9 @@ public sealed record ReminderInfo( bool Enabled, string? AgentDefinitionId, TrustAudience? Audience, - DateTimeOffset? ExpiresAt = null) : INoSerializationVerificationNeeded; + DateTimeOffset? ExpiresAt = null, + int ConsecutiveFailures = 0, + ReminderTerminalOutcome? TerminalOutcome = null) : INoSerializationVerificationNeeded; // ── Internal messages ── @@ -453,7 +473,25 @@ internal sealed record ReminderExecutionCompleted( Guid ExecutionId, ReminderId Id, bool Success, - string? ErrorMessage = null) : INoSerializationVerificationNeeded; + string? ErrorMessage = null, + bool OccurrenceTerminal = false) : INoSerializationVerificationNeeded; + +internal sealed record ReminderExecutionTerminated( + Guid ExecutionId, + ReminderId Id) : INoSerializationVerificationNeeded; + +/// +/// Durable state for the most relevant Akka.Reminders occurrence. +/// +public sealed record ReminderOccurrenceInfo( + DateTimeOffset DueTimeUtc, + DateTimeOffset? NextAttemptAtUtc, + int AttemptCount, + string? LastFailureReason, + string CompletionStatus, + DateTimeOffset? DeliveryDeadlineUtc, + DateTimeOffset? AckDeadlineUtc, + DateTimeOffset? CompletedAtUtc) : INoSerializationVerificationNeeded; // ── Execution history ── diff --git a/src/Netclaw.Cli/Reminder/ReminderCommand.cs b/src/Netclaw.Cli/Reminder/ReminderCommand.cs index 3acc3301c..dbcaa4c23 100644 --- a/src/Netclaw.Cli/Reminder/ReminderCommand.cs +++ b/src/Netclaw.Cli/Reminder/ReminderCommand.cs @@ -592,7 +592,18 @@ private static async Task RunStatusAsync(DaemonApi api, string[] args) Console.WriteLine($"Executing now: {status.Executing}"); Console.WriteLine($"Next fire: {status.NextFire ?? "not scheduled"}"); Console.WriteLine($"Consecutive fails: {status.ConsecutiveFailures}"); - Console.WriteLine($"Skipped (duplicate): {status.SkippedDuplicates}"); + Console.WriteLine($"Deferred overlaps: {status.SkippedDuplicates}"); + Console.WriteLine($"Terminal outcome: {status.TerminalOutcome ?? "none"}"); + + if (status.Occurrence is { } occurrence) + { + Console.WriteLine($"Occurrence status: {occurrence.CompletionStatus}"); + Console.WriteLine($"Occurrence attempts: {occurrence.AttemptCount}"); + if (occurrence.NextAttemptAtUtc is { } nextAttemptAtUtc) + Console.WriteLine($"Next retry: {nextAttemptAtUtc:u}"); + if (!string.IsNullOrWhiteSpace(occurrence.LastFailureReason)) + Console.WriteLine($"Last failure: {occurrence.LastFailureReason}"); + } var history = status.RecentHistory ?? []; if (history.Length == 0) @@ -630,8 +641,17 @@ private sealed record ReminderStatusView( string? NextFire, int ConsecutiveFailures, int SkippedDuplicates, + string? TerminalOutcome, + ReminderOccurrenceView? Occurrence, HistoryRecord[]? RecentHistory); + private sealed record ReminderOccurrenceView( + DateTimeOffset DueTimeUtc, + DateTimeOffset? NextAttemptAtUtc, + int AttemptCount, + string? LastFailureReason, + string CompletionStatus); + private static int WriteHelp() { Console.WriteLine("Usage: netclaw reminder "); @@ -647,7 +667,7 @@ private static int WriteHelp() Console.WriteLine(" validate Validate reminder file"); Console.WriteLine(" show Show reminder details"); Console.WriteLine(" history [--last N] Show recent execution history (default: 20)"); - Console.WriteLine(" status Show operational status: failures, skipped fires, in-flight"); + Console.WriteLine(" status Show execution, retry, terminal, and history status"); Console.WriteLine(); Console.WriteLine("Create options:"); Console.WriteLine(" --name Human-readable title (defaults to <id>)"); diff --git a/src/Netclaw.Daemon/Program.cs b/src/Netclaw.Daemon/Program.cs index cd2fa3ac9..69879f144 100644 --- a/src/Netclaw.Daemon/Program.cs +++ b/src/Netclaw.Daemon/Program.cs @@ -540,10 +540,8 @@ static void ConfigureDaemonServices( services.AddSingleton(effectivePolicyDefaults); services.AddSingleton<TrustContextDeriver>(); - // Reminders — no config surface exposed. Settings live as private - // consts on ReminderManagerActor / ReminderExecutionActor / - // ReminderScheduleParser / ReminderHistoryStore. Library defaults - // cover AckTimeout, MaxRetryBackoff, and MaxDeliveryAttempts. + // Reminder limits stay private. Netclaw sets the library acknowledgement + // lease in WithReminderManager because an LLM attempt can take one hour. services.AddSingleton<ReminderDefinitionStore>(); services.AddSingleton<ReminderHistoryStore>(); diff --git a/src/Netclaw.Daemon/Reminders/ReminderEndpointRouteBuilderExtensions.cs b/src/Netclaw.Daemon/Reminders/ReminderEndpointRouteBuilderExtensions.cs index c9c47d906..40a335e54 100644 --- a/src/Netclaw.Daemon/Reminders/ReminderEndpointRouteBuilderExtensions.cs +++ b/src/Netclaw.Daemon/Reminders/ReminderEndpointRouteBuilderExtensions.cs @@ -43,7 +43,9 @@ public static IEndpointRouteBuilder MapReminderEndpoints(this IEndpointRouteBuil ExpiresAt: r.ExpiresAt is null ? null : SetReminderTool.FormatTimestamp(r.ExpiresAt), - Audience: r.Audience?.ToWireValue())); + Audience: r.Audience?.ToWireValue(), + ConsecutiveFailures: r.ConsecutiveFailures, + TerminalOutcome: r.TerminalOutcome?.ToString())); return TypedResults.Ok(projected); }) .WithName("ListReminders") @@ -287,7 +289,9 @@ public static IEndpointRouteBuilder MapReminderEndpoints(this IEndpointRouteBuil DeliveryAddress: r.Delivery.Address, DeliveryRequired: r.DeliveryRequired, DeliveryInstructions: r.DeliveryInstructions, - Audience: r.Audience?.ToWireValue())); + Audience: r.Audience?.ToWireValue(), + ConsecutiveFailures: r.ConsecutiveFailures, + TerminalOutcome: r.TerminalOutcome?.ToString())); }) .WithName("GetReminder") .WithSummary("Get a single reminder's full definition."); @@ -331,10 +335,12 @@ public static IEndpointRouteBuilder MapReminderEndpoints(this IEndpointRouteBuil NextFire: status.NextFire is null ? null : SetReminderTool.FormatTimestamp(status.NextFire), ConsecutiveFailures: status.ConsecutiveFailures, SkippedDuplicates: status.SkippedDuplicates, + TerminalOutcome: status.TerminalOutcome?.ToString(), + Occurrence: status.Occurrence, RecentHistory: status.RecentHistory)); }) .WithName("GetReminderStatus") - .WithSummary("Get per-reminder operational status: in-flight, consecutive failures, skipped fires, recent history."); + .WithSummary("Get reminder execution, retry, failure, terminal, and history status."); return app; } @@ -392,7 +398,9 @@ internal sealed record ReminderSummaryDto( string Schedule, string NextFire, string? ExpiresAt, - string? Audience); + string? Audience, + int ConsecutiveFailures, + string? TerminalOutcome); /// <summary>Full reminder projection returned by <c>GET /api/reminders/{id}</c>.</summary> internal sealed record ReminderDetailDto( @@ -408,7 +416,9 @@ internal sealed record ReminderDetailDto( string? DeliveryAddress, bool DeliveryRequired, string? DeliveryInstructions, - string? Audience); + string? Audience, + int ConsecutiveFailures, + string? TerminalOutcome); /// <summary>Per-reminder operational status projection (see <c>GET /{id}/status</c>).</summary> internal sealed record ReminderStatusDto( @@ -418,6 +428,8 @@ internal sealed record ReminderStatusDto( string? NextFire, int ConsecutiveFailures, int SkippedDuplicates, + string? TerminalOutcome, + ReminderOccurrenceInfo? Occurrence, IReadOnlyList<HistoryRecord> RecentHistory); /// <summary>Acknowledgement carrying a human-readable message.</summary> diff --git a/src/Netclaw.Daemon/Webhooks/WebhookExecutionActor.cs b/src/Netclaw.Daemon/Webhooks/WebhookExecutionActor.cs index 33ec0bc07..d7b4978e9 100644 --- a/src/Netclaw.Daemon/Webhooks/WebhookExecutionActor.cs +++ b/src/Netclaw.Daemon/Webhooks/WebhookExecutionActor.cs @@ -51,6 +51,7 @@ public WebhookExecutionActor( Receive<ExecutionStarted>(_ => { }); Receive<ExecutionOutput>(HandleOutput); + Receive<OutputStreamTerminated>(HandleOutputStreamTerminated); Receive<ReceiveTimeout>(_ => { _log.Warning( @@ -83,7 +84,8 @@ private async Task InitializeAsync() Filter = OutputFilter.TextStreaming | OutputFilter.ToolCalls, PromptOverlay = _invocation.Route.BuildPromptOverlay() }, - output => self.Tell(new ExecutionOutput(output))); + output => self.Tell(new ExecutionOutput(output)), + failure => self.Tell(new OutputStreamTerminated(failure))); await inputQueue.OfferAsync(new ChannelInput { @@ -137,6 +139,15 @@ private void HandleOutput(ExecutionOutput wrapper) } } + private void HandleOutputStreamTerminated(OutputStreamTerminated terminated) + { + if (_completed) + return; + + var reason = terminated.Failure?.Message ?? "Session output ended without a terminal result."; + ReportAndStop(false, reason); + } + private void ReportAndStop(bool success, string? errorMessage) { if (_completed) @@ -182,4 +193,5 @@ private static PayloadTaint ToPayloadTaint(TrustAudience audience) private sealed record ExecutionStarted; private sealed record ExecutionOutput(SessionOutput Output); + private sealed record OutputStreamTerminated(Exception? Failure); } From c207cecefef4f5da8d93b619df3f192d3308838c Mon Sep 17 00:00:00 2001 From: Aaron Stannard <aaron@petabridge.com> Date: Sat, 8 Aug 2026 01:09:34 +0000 Subject: [PATCH 2/4] fix(reminders): coordinate occurrence settlement --- .../.system/files/netclaw-operations/SKILL.md | 2 +- .../references/scheduling.md | 9 +- .../design.md | 25 +- .../proposal.md | 2 + .../specs/netclaw-scheduling/spec.md | 162 +++++ .../specs/reminder-execution-history/spec.md | 7 + .../reliable-one-shot-reminder-retry/tasks.md | 9 + .../Reminders/ReminderExecutionActorTests.cs | 67 +- .../Reminders/ReminderManagerActorTests.cs | 224 +++++-- .../Reminders/ActiveExecutionTracker.cs | 11 +- .../Reminders/ReminderExecutionActor.cs | 209 ++---- .../Reminders/ReminderManagerActor.cs | 618 +++++++++++------- .../Reminders/ReminderProtocol.cs | 252 +++---- src/Netclaw.Cli/Reminder/ReminderCommand.cs | 4 +- .../ReminderEndpointAuthorizationTests.cs | 67 ++ 15 files changed, 1097 insertions(+), 571 deletions(-) diff --git a/feeds/skills/.system/files/netclaw-operations/SKILL.md b/feeds/skills/.system/files/netclaw-operations/SKILL.md index e2481b02b..c14e99532 100644 --- a/feeds/skills/.system/files/netclaw-operations/SKILL.md +++ b/feeds/skills/.system/files/netclaw-operations/SKILL.md @@ -3,7 +3,7 @@ name: netclaw-operations description: "REQUIRED when the user asks about scheduling, reminders, cron jobs, timers, background jobs, diagnostics, troubleshooting, MCP tools, daemon health, identity updates, or Netclaw capabilities and self-maintenance." metadata: author: netclaw - version: "2.41.0" + version: "2.42.0" --- # Netclaw Operations diff --git a/feeds/skills/.system/files/netclaw-operations/references/scheduling.md b/feeds/skills/.system/files/netclaw-operations/references/scheduling.md index 066ad7ef6..b68fe9266 100644 --- a/feeds/skills/.system/files/netclaw-operations/references/scheduling.md +++ b/feeds/skills/.system/files/netclaw-operations/references/scheduling.md @@ -82,16 +82,17 @@ as a plain-language notice to the reminder's **destination channel** (for reminder's output. This is bounded by the auto-disable threshold (at most a few notices plus the disabled notice), not the unbounded skip stream. -A second fire waits in the deferred queue while the prior execution runs. -Netclaw counts this event but does not post it to the channel. The status command -shows the count: +A one-shot that cannot start receives a negative acknowledgement. Akka.Reminders +then controls its retry delay. Netclaw acknowledges and skips a blocked recurring +occurrence. It does not keep a stale catch-up queue. The status command shows the +skip count: ``` netclaw reminder status <id> ``` `status` shows the enabled state, the terminal outcome, and current execution -state. It also shows the next fire, consecutive failures, deferred overlap count, +state. It also shows the next fire, consecutive failures, skipped occurrence count, and recent history. For one-shots, it shows the durable occurrence state, attempt count, next retry time, and last failure reason. diff --git a/openspec/changes/reliable-one-shot-reminder-retry/design.md b/openspec/changes/reliable-one-shot-reminder-retry/design.md index e572b0596..8744dafd0 100644 --- a/openspec/changes/reliable-one-shot-reminder-retry/design.md +++ b/openspec/changes/reliable-one-shot-reminder-retry/design.md @@ -33,11 +33,27 @@ Netclaw persists `ConsecutiveFailures` in each reminder definition. Each failed This value spans recurring occurrences. Akka.Reminders resets its attempt count for each new occurrence. -### Every delivery mode delays the acknowledgment +### The reminder manager coordinates settlement -`ReminderManagerActor` passes the envelope to `ReminderExecutionActor` for all delivery kinds. The child acknowledges only after execution and required delivery succeed. +`ReminderManagerActor` passes the envelope to `ReminderExecutionActor` for all delivery kinds. The child reports its outcome and waits for manager acceptance. -The child sends a negative acknowledgement after a known failure. An actor crash leaves the occurrence unacknowledged, so the library timeout remains the final recovery path. +The manager saves the history and reminder state before it settles a known failure. It then sends the negative acknowledgement. + +The manager resets the reminder failure count before it acknowledges a success. It records one-shot completion only after a successful acknowledgement. + +The manager replies to the child after settlement. The child stops only after this reply, so DeathWatch cannot replace an accepted result. + +An actor crash before an outcome leaves the occurrence unacknowledged. The manager records the crash and attempts a negative acknowledgement without risking its own lifecycle. + +### Capacity does not transfer occurrence ownership + +Netclaw does not retain a blocked Akka.Reminders envelope in an in-memory queue. A queue wait could consume the 70-minute acknowledgement lease. + +Netclaw negatively acknowledges a blocked one-shot. Akka.Reminders then owns its retry delay and attempt budget. + +Netclaw acknowledges and skips a blocked reminder-series occurrence. This rule prevents a catch-up queue and preserves the latest-only series policy. + +Netclaw ignores an exact duplicate of the active occurrence. The active execution remains the sole settlement owner. ### One-shot completion uses a soft delete @@ -55,10 +71,13 @@ Netclaw sets the Akka acknowledgment timeout to 70 minutes. The execution actor Known failures use negative acknowledgement and do not wait for the acknowledgment timeout. +Netclaw starts an attempt only when the remaining envelope lease exceeds the maximum attempt duration plus a settlement margin. + ## Risks / Trade-offs - **A daemon crash can delay retry for 70 minutes.** The long lease prevents duplicate LLM work during a valid one-hour attempt. - **At-least-once delivery can duplicate work.** The occurrence identity remains `(Entity, Key, DueTimeUtc)` and session identifiers use that stable due time. +- **Netclaw and Akka.Reminders use separate stores.** Ordered writes and reconciliation provide convergence without a cross-store transaction. - **A custom Akka storage provider can lack status queries.** Netclaw uses the official SQLite provider and fails loudly if the capability is absent. - **Old JSON files lack the new fields.** Serializer defaults preserve active state and a zero failure count. diff --git a/openspec/changes/reliable-one-shot-reminder-retry/proposal.md b/openspec/changes/reliable-one-shot-reminder-retry/proposal.md index 0bcea8281..db36755a5 100644 --- a/openspec/changes/reliable-one-shot-reminder-retry/proposal.md +++ b/openspec/changes/reliable-one-shot-reminder-retry/proposal.md @@ -9,6 +9,8 @@ PRD-008 requires durable failure records and an automatic pause after repeated f - A failed one-shot will remain enabled while another occurrence attempt is pending. - A completed or terminally failed one-shot will use a soft delete. - Netclaw will persist its reminder-level consecutive failure count. +- The reminder manager will coordinate local state and Akka occurrence settlement. +- Netclaw will not keep Akka.Reminders envelopes in an in-memory catch-up queue. - Reconciliation will use durable occurrence state and will never infer success from a past due time. - Reminder status output will show the durable occurrence attempt and terminal outcome. diff --git a/openspec/changes/reliable-one-shot-reminder-retry/specs/netclaw-scheduling/spec.md b/openspec/changes/reliable-one-shot-reminder-retry/specs/netclaw-scheduling/spec.md index 6076b5761..3387f6bb6 100644 --- a/openspec/changes/reliable-one-shot-reminder-retry/specs/netclaw-scheduling/spec.md +++ b/openspec/changes/reliable-one-shot-reminder-retry/specs/netclaw-scheduling/spec.md @@ -1,3 +1,107 @@ +## MODIFIED Requirements + +### Requirement: Failure handling and guardrails + +The reminder manager SHALL store consecutive failures in each reminder definition. A successful execution SHALL reset the count. + +The manager SHALL disable a reminder when the count reaches `FailurePauseThreshold`. The disabled definition SHALL remain available for status and diagnosis. + +The manager SHALL enforce `MaxConcurrentExecutions`. It SHALL apply the bounded capacity policy when no execution slot is available. + +Each execution SHALL have a one-hour absolute limit. A known timeout SHALL count as a failed attempt. + +#### Scenario: Consecutive failures disable a reminder + +- **GIVEN** a reminder has one fewer failure than `FailurePauseThreshold` +- **WHEN** its next execution fails +- **THEN** the manager saves the threshold failure count +- **AND** the manager disables the reminder +- **AND** the definition remains available + +#### Scenario: A successful execution resets the failure count + +- **GIVEN** a reminder has one or more consecutive failures +- **WHEN** its next execution succeeds +- **THEN** the manager saves a zero failure count + +#### Scenario: The execution limit is full + +- **GIVEN** `MaxConcurrentExecutions` reminder attempts are active +- **WHEN** another occurrence arrives +- **THEN** the manager does not retain the envelope in a queue +- **AND** the manager applies the one-shot or reminder-series capacity policy + +### Requirement: Envelope-ack-gated at-least-once delivery for Mode B + +The reminder manager SHALL retain each Akka.Reminders envelope until the attempt has a known outcome. This rule SHALL apply to every delivery kind. + +The execution actor SHALL report its outcome to the manager. It SHALL wait for `ReminderExecutionAccepted` before it stops. + +The manager SHALL acknowledge only a successful execution with all required delivery evidence. It SHALL negatively acknowledge a known failure. + +A `CurrentSession` reminder SHALL still use the origin gateway and `Ask<CommandAck>`. Required delivery SHALL also wait for `ReminderDeliveryResult`. + +The target session SHALL keep its best-effort reminder key check. The key SHALL use the stable occurrence due time. + +#### Scenario: CurrentSession requires observed delivery + +- **GIVEN** a `CurrentSession` reminder has `DeliveryRequired = true` +- **WHEN** the target session returns `CommandAck` +- **THEN** the execution remains incomplete +- **WHEN** a matching successful `ReminderDeliveryResult` arrives +- **THEN** the child reports success to the manager +- **AND** the manager acknowledges the occurrence + +#### Scenario: CurrentSession delivery fails + +- **GIVEN** a `CurrentSession` reminder awaits required delivery +- **WHEN** the gateway rejects the turn or delivery fails +- **THEN** the child reports a descriptive failure +- **AND** the manager sends a negative acknowledgement + +#### Scenario: Channel execution fails + +- **GIVEN** a `Channel` reminder starts an isolated execution +- **WHEN** the execution or notification fails +- **THEN** the manager does not acknowledge success +- **AND** the manager sends a negative acknowledgement + +#### Scenario: None delivery succeeds + +- **GIVEN** a reminder uses `Delivery.Kind = None` +- **WHEN** its execution completes successfully +- **THEN** the manager acknowledges the occurrence + +#### Scenario: The child reports success before it stops + +- **GIVEN** an execution child reports success +- **WHEN** the manager saves local state and acknowledges the occurrence +- **THEN** the manager sends `ReminderExecutionAccepted` +- **AND** the child stops after that message + +### Requirement: Reminder delivery guarantees + +The reminder pipeline SHALL provide at-least-once attempt delivery until the manager confirms execution and required delivery success. + +A crash before acknowledgement SHALL leave the occurrence eligible for retry. A crash after acknowledgement SHALL not lose successful work. + +The stable occurrence identity and the session reminder key SHALL reduce duplicate work. Netclaw SHALL not claim exactly-once delivery. + +#### Scenario: The daemon stops during execution + +- **GIVEN** a reminder attempt has not reached manager acknowledgement +- **WHEN** the daemon stops +- **THEN** the acknowledgement lease expires +- **AND** Akka.Reminders can retry the occurrence + +#### Scenario: The daemon stops after acknowledgement + +- **GIVEN** execution and required delivery succeeded +- **AND** the manager acknowledged the occurrence +- **WHEN** the daemon stops before one-shot terminal state is saved +- **THEN** durable occurrence status remains `Delivered` +- **AND** reconciliation repairs the one-shot terminal state + ## ADDED Requirements ### Requirement: Execution outcome controls occurrence acknowledgement @@ -6,6 +110,8 @@ Netclaw SHALL pass the Akka.Reminders envelope to every reminder execution. Netc Netclaw SHALL send a negative acknowledgement after a known execution or delivery failure. The negative acknowledgement SHALL use the library retry budget. +The reminder manager SHALL accept the execution result before the child stops. DeathWatch SHALL report failure only before result acceptance. + #### Scenario: Channel execution fails before delivery - **GIVEN** an enabled channel reminder occurrence is awaiting acknowledgement @@ -91,3 +197,59 @@ Netclaw SHALL use a one-hour absolute execution limit and a 70-minute Akka.Remin - **WHEN** the absolute limit expires - **THEN** Netclaw stops the attempt - **AND** Netclaw sends a negative acknowledgement + +#### Scenario: The remaining lease cannot contain an attempt + +- **GIVEN** an occurrence has less than the maximum attempt duration plus the settlement margin remaining +- **WHEN** Netclaw considers the occurrence for execution +- **THEN** Netclaw does not start the execution +- **AND** Netclaw settles the occurrence by its one-shot or reminder-series capacity policy + +### Requirement: Capacity settlement remains bounded + +Netclaw SHALL NOT retain blocked Akka.Reminders envelopes in an in-memory catch-up queue. + +Netclaw SHALL negatively acknowledge a blocked one-shot occurrence. Netclaw SHALL acknowledge and skip a blocked reminder-series occurrence. + +Netclaw SHALL ignore an exact duplicate of the active occurrence. The active execution SHALL remain the sole settlement owner. + +#### Scenario: One-shot execution capacity is unavailable + +- **GIVEN** a one-shot occurrence cannot start because execution capacity is full +- **WHEN** the manager handles the occurrence +- **THEN** the manager sends a negative acknowledgement +- **AND** Akka.Reminders owns the retry delay + +#### Scenario: Reminder-series execution capacity is unavailable + +- **GIVEN** a reminder-series occurrence cannot start because execution capacity is full +- **WHEN** the manager handles the occurrence +- **THEN** the manager acknowledges the occurrence without execution +- **AND** Netclaw does not retain the occurrence for catch-up work + +#### Scenario: Exact active occurrence arrives again + +- **GIVEN** an occurrence already has an active execution +- **WHEN** the same key, due time, and acknowledgement deadline arrive again +- **THEN** Netclaw does not start or settle the duplicate envelope +- **AND** the active execution remains the sole settlement owner + +### Requirement: Settlement write order supports recovery + +Netclaw SHALL save a failed run and its poison count before it sends a negative acknowledgement. Netclaw SHALL not advance Akka state after a local save failure. + +Netclaw SHALL save a successful run and reset the poison count before it sends an acknowledgement. Reconciliation SHALL repair one-shot terminal state after a post-acknowledgement process failure. + +#### Scenario: Local failure state cannot be saved + +- **GIVEN** an execution attempt fails +- **WHEN** Netclaw cannot save its poison state +- **THEN** Netclaw does not send a negative acknowledgement +- **AND** the Akka.Reminders acknowledgement timeout remains the recovery path + +#### Scenario: Process stops after successful acknowledgement + +- **GIVEN** Netclaw acknowledges a successful one-shot +- **WHEN** the process stops before it saves the terminal outcome +- **THEN** reconciliation reads the durable delivered state +- **AND** reconciliation records the completed soft delete diff --git a/openspec/changes/reliable-one-shot-reminder-retry/specs/reminder-execution-history/spec.md b/openspec/changes/reliable-one-shot-reminder-retry/specs/reminder-execution-history/spec.md index c61208697..38c60f084 100644 --- a/openspec/changes/reliable-one-shot-reminder-retry/specs/reminder-execution-history/spec.md +++ b/openspec/changes/reliable-one-shot-reminder-retry/specs/reminder-execution-history/spec.md @@ -15,3 +15,10 @@ Netclaw SHALL retain execution history when it soft-deletes a completed or faile - **GIVEN** a one-shot reaches its poison threshold - **WHEN** Netclaw disables it with outcome `Failed` - **THEN** all failure records remain available through reminder history + +#### Scenario: Execution actor stops before it reports an outcome + +- **GIVEN** a reminder execution actor stops before manager acceptance +- **WHEN** DeathWatch reports the stop +- **THEN** the manager appends a failed execution record +- **AND** the failure record identifies the unexpected stop diff --git a/openspec/changes/reliable-one-shot-reminder-retry/tasks.md b/openspec/changes/reliable-one-shot-reminder-retry/tasks.md index eb763dcea..520af782d 100644 --- a/openspec/changes/reliable-one-shot-reminder-retry/tasks.md +++ b/openspec/changes/reliable-one-shot-reminder-retry/tasks.md @@ -23,3 +23,12 @@ - [x] 4.1 Add actor tests for retry, later success, poison pause, restart state, and reconciliation retention. - [x] 4.2 Update the `netclaw-operations` system skill and its version. - [x] 4.3 Run focused tests, the full affected suites, evals, Slopwatch, and file-header verification. + +## 5. Adversarial review corrections + +- [x] 5.1 Move Ack and Nack coordination to the reminder manager and add a child completion handshake. +- [x] 5.2 Remove deferred envelope retention and apply explicit one-shot, reminder-series, duplicate, and lease policies. +- [x] 5.3 Preserve terminal diagnostics after a failed enable request and record unexpected actor termination history. +- [x] 5.4 Order local state writes before occurrence settlement and add recovery behavior for settlement faults. +- [x] 5.5 Add actor, restart, lease, capacity, history, and endpoint regression tests. +- [ ] 5.6 Run focused tests, the full solution, evals, Slopwatch, and file-header verification. diff --git a/src/Netclaw.Actors.Tests/Reminders/ReminderExecutionActorTests.cs b/src/Netclaw.Actors.Tests/Reminders/ReminderExecutionActorTests.cs index c3d2beaf6..e5af89e72 100644 --- a/src/Netclaw.Actors.Tests/Reminders/ReminderExecutionActorTests.cs +++ b/src/Netclaw.Actors.Tests/Reminders/ReminderExecutionActorTests.cs @@ -1,4 +1,4 @@ -// ----------------------------------------------------------------------- +// ----------------------------------------------------------------------- // <copyright file="ReminderExecutionActorTests.cs" company="Petabridge, LLC"> // Copyright (C) 2026 - 2026 Petabridge, LLC <https://petabridge.com> // </copyright> @@ -20,6 +20,7 @@ namespace Netclaw.Actors.Tests.Reminders; +[Collection(ReminderActorTestCollection.Name)] public class ReminderExecutionActorTests : TestKit, IDisposable { private readonly DisposableTempDir _dir = new(); @@ -63,6 +64,39 @@ public async Task Execution_failure_reports_completed_false_with_error_message() Assert.Equal("outer failure", completed.ErrorMessage); } + [Fact] + public async Task Execution_waits_for_manager_acceptance_before_stop() + { + var pipeline = new FailingSessionPipeline(new InvalidOperationException("failed")); + var definition = CreateDefinition("settlement-handshake"); + var probe = CreateTestProbe(); + Sys.ActorOf( + Props.Create(() => new ParentProxy( + probe.Ref, + definition, + pipeline, + _historyStore, + acceptCompletion: false, + reportChild: true)), + "exec-handshake-parent"); + + var child = (await probe.ExpectMsgAsync<ChildCreated>( + TimeSpan.FromSeconds(5), + cancellationToken: TestContext.Current.CancellationToken)).Child; + Watch(child); + + var completed = await probe.ExpectMsgAsync<ReminderExecutionCompleted>( + TimeSpan.FromSeconds(5), + cancellationToken: TestContext.Current.CancellationToken); + await ExpectNoMsgAsync(TimeSpan.FromMilliseconds(250), TestContext.Current.CancellationToken); + + child.Tell(new ReminderExecutionAccepted(completed.ExecutionId)); + await ExpectTerminatedAsync( + child, + TimeSpan.FromSeconds(5), + cancellationToken: TestContext.Current.CancellationToken); + } + [Fact] public async Task Execution_failure_with_inner_exception_propagates_outer_message() { @@ -370,22 +404,41 @@ public ParentProxy( IActorRef probe, ReminderDefinition definition, ISessionPipeline pipeline, - ReminderHistoryStore historyStore) + ReminderHistoryStore historyStore, + bool acceptCompletion = true, + bool reportChild = false) { var executionId = Guid.NewGuid(); - Context.ActorOf( + var envelope = new Akka.Reminders.ReminderEnvelope<ReminderPayload>( + new Akka.Reminders.ReminderEntity(ReminderManagerActor.ShardRegionName, ReminderManagerActor.EntityId), + new Akka.Reminders.ReminderKey(definition.Id.Value), + definition.Schedule.FireAt ?? TimeProvider.System.GetUtcNow(), + Akka.Reminders.ReminderDeadline.Infinite, + new ReminderPayload { Id = definition.Id }); + var child = Context.ActorOf( ReminderExecutionActor.CreateProps( executionId, definition, pipeline, TimeProvider.System, - historyStore), + envelope), "exec"); + if (reportChild) + probe.Tell(new ChildCreated(child)); + ReceiveAsync<ReminderExecutionCompleted>(async completed => + { + await historyStore.AppendAsync(completed.Id, completed.History); + probe.Tell(completed); + if (acceptCompletion) + Sender.Tell(new ReminderExecutionAccepted(completed.ExecutionId)); + }); ReceiveAny(msg => probe.Tell(msg)); } } + private sealed record ChildCreated(IActorRef Child); + /// <summary>Fake pipeline that throws a pre-configured exception on CreateAsync.</summary> private sealed class FailingSessionPipeline(Exception exception) : ISessionPipeline { @@ -639,3 +692,9 @@ await AwaitConditionAsync( Assert.Equal("pipeline blew up", records[0].ErrorMessage); } } + +[CollectionDefinition(Name)] +public sealed class ReminderActorTestCollection +{ + public const string Name = "Reminder actor tests"; +} diff --git a/src/Netclaw.Actors.Tests/Reminders/ReminderManagerActorTests.cs b/src/Netclaw.Actors.Tests/Reminders/ReminderManagerActorTests.cs index f584f3753..5a1772a13 100644 --- a/src/Netclaw.Actors.Tests/Reminders/ReminderManagerActorTests.cs +++ b/src/Netclaw.Actors.Tests/Reminders/ReminderManagerActorTests.cs @@ -1,4 +1,4 @@ -// ----------------------------------------------------------------------- +// ----------------------------------------------------------------------- // <copyright file="ReminderManagerActorTests.cs" company="Petabridge, LLC"> // Copyright (C) 2026 - 2026 Petabridge, LLC <https://petabridge.com> // </copyright> @@ -23,10 +23,12 @@ namespace Netclaw.Actors.Tests.Reminders; +[Collection(ReminderActorTestCollection.Name)] public class ReminderManagerActorTests : TestKit { private readonly string _basePath = Path.Combine(Path.GetTempPath(), $"netclaw-reminder-tests-{Guid.NewGuid():N}"); private readonly FakeTimeProvider _timeProvider = new(TimeProvider.System.GetUtcNow()); + private readonly TestShardRegionResolver _sharedResolver = new(); private ReminderDefinitionStore _definitionStore = null!; private TestNotificationSink _notificationSink = null!; private readonly FailingReminderSessionPipeline _sessionPipeline = @@ -50,14 +52,13 @@ protected override void ConfigureAkka(AkkaConfigurationBuilder builder, IService var historyStore = new ReminderHistoryStore(paths); // Wire local reminders with in-memory storage - var sharedResolver = new TestShardRegionResolver(); builder.WithLocalReminders(reminders => { reminders.WithInMemoryStorage(); - reminders.WithResolver(_ => sharedResolver); + reminders.WithResolver(_ => _sharedResolver); reminders.WithSettings(new ReminderSettings { - AckTimeout = TimeSpan.FromSeconds(2), + AckTimeout = TimeSpan.FromMinutes(70), RetryBackoffBase = TimeSpan.FromMilliseconds(25), MaxRetryBackoff = TimeSpan.FromMilliseconds(25), MaxDeliveryAttempts = 10 @@ -68,25 +69,32 @@ protected override void ConfigureAkka(AkkaConfigurationBuilder builder, IService { registry.Register<SessionManagerActorKey>(system.DeadLetters); - var defaults = new EffectivePolicyDefaults( - DeploymentPosture.Team, TrustAudience.Team, ShellExecutionMode.Off, false); var reminderManager = system.ActorOf( - Props.Create(() => new ReminderManagerActor( - _sessionPipeline, - defaults, - new SchedulingConfig(), - _timeProvider, - definitionStore, - historyStore, - _notificationSink, - NullReminderChannelNotifier.Instance)), + CreateManagerProps(definitionStore, historyStore), "reminder-manager-test"); registry.Register<ReminderManagerActorKey>(reminderManager); - sharedResolver.RegisterShardRegion(ReminderManagerActor.ShardRegionName, reminderManager); + _sharedResolver.RegisterShardRegion(ReminderManagerActor.ShardRegionName, reminderManager); }); } + private Props CreateManagerProps( + ReminderDefinitionStore definitionStore, + ReminderHistoryStore historyStore) + { + var defaults = new EffectivePolicyDefaults( + DeploymentPosture.Team, TrustAudience.Team, ShellExecutionMode.Off, false); + return Props.Create(() => new ReminderManagerActor( + _sessionPipeline, + defaults, + new SchedulingConfig(), + _timeProvider, + definitionStore, + historyStore, + _notificationSink, + NullReminderChannelNotifier.Instance)); + } + private async Task<IActorRef> GetManagerAsync() { var registry = ActorRegistry.For(Sys); @@ -237,6 +245,81 @@ public async Task Status_query_reads_durable_failure_count_after_store_reload() Assert.Equal(3, status.ConsecutiveFailures); } + [Fact] + public async Task Failed_enable_keeps_terminal_diagnostics() + { + var manager = await GetManagerAsync(); + var definition = CreateDefinition("failed-enable", "Failed enable") with + { + Enabled = false, + ConsecutiveFailures = 5, + TerminalOutcome = ReminderTerminalOutcome.Failed, + Schedule = new ReminderSchedule + { + Type = ReminderScheduleType.OneShot, + FireAt = TimeProvider.System.GetUtcNow().AddMinutes(-1) + } + }; + _definitionStore.Save(definition); + + var response = await manager.Ask<ReminderStateResponse>( + new EnableReminderCommand(definition.Id), + TimeSpan.FromSeconds(5), + TestContext.Current.CancellationToken); + + Assert.True(response.Found); + Assert.False(response.Enabled); + Assert.NotNull(response.ErrorMessage); + var stored = _definitionStore.Get(definition.Id); + Assert.NotNull(stored); + Assert.False(stored!.Enabled); + Assert.Equal(5, stored.ConsecutiveFailures); + Assert.Equal(ReminderTerminalOutcome.Failed, stored.TerminalOutcome); + } + + [Fact] + public async Task Manager_restart_preserves_definition_and_occurrence_state() + { + var manager = await GetManagerAsync(); + var definition = CreateDefinition("restart-state", "Restart state") with + { + ConsecutiveFailures = 3 + }; + var saved = await manager.Ask<ReminderSavedResponse>( + new SaveReminderCommand( + definition, + Authorization: new ReminderAudienceAuthorizationContext(TrustAudience.Team, "test")), + TimeSpan.FromSeconds(5), + TestContext.Current.CancellationToken); + Assert.True(saved.Success, saved.ErrorMessage); + + Watch(manager); + Sys.Stop(manager); + await ExpectTerminatedAsync( + manager, + TimeSpan.FromSeconds(5), + cancellationToken: TestContext.Current.CancellationToken); + + var paths = new NetclawPaths(_basePath); + var restarted = Sys.ActorOf( + CreateManagerProps( + new ReminderDefinitionStore(paths), + new ReminderHistoryStore(paths)), + "reminder-manager-restarted"); + ActorRegistry.For(Sys).Register<ReminderManagerActorKey>(restarted, overwrite: true); + _sharedResolver.RegisterShardRegion(ReminderManagerActor.ShardRegionName, restarted); + + var status = await restarted.Ask<ReminderStatusResponse>( + new GetReminderStatusQuery(definition.Id), + TimeSpan.FromSeconds(5), + TestContext.Current.CancellationToken); + Assert.True(status.Found); + Assert.True(status.Enabled); + Assert.Equal(3, status.ConsecutiveFailures); + Assert.NotNull(status.NextFire); + Assert.Equal("Pending", status.Occurrence?.CompletionStatus); + } + [Fact] public async Task Reconcile_retains_past_oneshot_without_durable_outcome() { @@ -931,7 +1014,7 @@ await AwaitAssertAsync(async () => } [Fact] - public async Task Deferred_expired_recurring_reminder_is_disabled_before_execution() + public async Task Recurring_occurrence_at_capacity_is_acked_without_execution() { var manager = await GetManagerAsync(); @@ -941,10 +1024,10 @@ public async Task Deferred_expired_recurring_reminder_is_disabled_before_executi await manager.Ask<ReminderManagerActor.ReconcileCompleted>( ReminderManagerActor.ReconcileReminders.Instance, TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken); - var gatewayProbe = CreateTestProbe("deferred-expiry-gateway"); + var gatewayProbe = CreateTestProbe("capacity-gateway"); var autoAckRef = Sys.ActorOf( Props.Create(() => new AutoAckTrustedGateway(gatewayProbe.Ref)), - "auto-ack-deferred-expiry"); + "auto-ack-capacity"); ActorRegistry.For(Sys).Register<SlackGatewayActorKey>(autoAckRef); // Save before dispatch so filesystem latency cannot consume any test @@ -958,7 +1041,6 @@ public async Task Deferred_expired_recurring_reminder_is_disabled_before_executi for (var i = 0; i < ReminderManagerActor.MaxConcurrentExecutions; i++) manager.Tell(CreateEnvelope($"blocking-{i}")); - var blockers = new List<DeliverTrustedSessionTurn>(); for (var i = 0; i < ReminderManagerActor.MaxConcurrentExecutions; i++) { var delivered = await gatewayProbe.ExpectMsgAsync<DeliverTrustedSessionTurn>( @@ -966,24 +1048,23 @@ public async Task Deferred_expired_recurring_reminder_is_disabled_before_executi cancellationToken: TestContext.Current.CancellationToken); Assert.NotNull(delivered.Source.ReminderId); Assert.NotNull(delivered.Source.DeliveryObserver); - blockers.Add(delivered); } - var now = _timeProvider.GetUtcNow(); - var expiringId = "queued-expiring"; - var expiringReminder = new ReminderDefinition + var invocationCount = _sessionPipeline.InvocationCount; + var now = TimeProvider.System.GetUtcNow(); + var recurringId = "capacity-recurring"; + var recurringReminder = new ReminderDefinition { - Id = new ReminderId(expiringId), - Title = "Queued expiring reminder", - Instructions = "Should not execute after expiry", + Id = new ReminderId(recurringId), + Title = "Capacity recurring reminder", + Instructions = "Do not create a stale catch-up execution", Delivery = new ReminderDelivery { Kind = DeliveryKind.None }, Schedule = new ReminderSchedule { Type = ReminderScheduleType.Interval, Interval = TimeSpan.FromMinutes(30), - FireAt = now.AddMinutes(30) + FireAt = now.AddMilliseconds(100) }, - ExpiresAt = now.AddMinutes(1), Audience = TrustAudience.Team, Boundary = TrustBoundary.Team, Enabled = true, @@ -991,38 +1072,36 @@ public async Task Deferred_expired_recurring_reminder_is_disabled_before_executi CreatedAt = now, UpdatedAt = now }; - _definitionStore.Save(expiringReminder); - - // The query comes from the same sender as the fire, so its reply is a - // mailbox-order barrier proving the fire was handled while all three - // blockers were still active. - var controlProbe = CreateTestProbe("deferred-expiry-control"); - controlProbe.Send(manager, CreateEnvelope(expiringId)); - controlProbe.Send(manager, GetReminderHealthQuery.Instance); - var health = await controlProbe.ExpectMsgAsync<ReminderHealthResponse>( + var saved = await manager.Ask<ReminderSavedResponse>( + new SaveReminderCommand( + recurringReminder, + Authorization: new ReminderAudienceAuthorizationContext(TrustAudience.Team, "test")), TimeSpan.FromSeconds(5), - cancellationToken: TestContext.Current.CancellationToken); - Assert.Equal(ReminderManagerActor.MaxConcurrentExecutions, health.ActiveExecutions); - - _timeProvider.Advance(TimeSpan.FromMinutes(2)); - - var blocker = blockers[0]; - blocker.Source.DeliveryObserver!.Tell(new ReminderDeliveryResult( - blocker.Source.ReminderId!.Value, - ChannelType.Slack, - Delivered: true, - ObservedAtMs: _timeProvider.GetUtcNow().ToUnixTimeMilliseconds())); + TestContext.Current.CancellationToken); + Assert.True(saved.Success, saved.ErrorMessage); - await AwaitAssertAsync(() => + await AwaitAssertAsync(async () => { - var stored = _definitionStore.Get(new ReminderId(expiringId)); - Assert.NotNull(stored); - Assert.False(stored!.Enabled); + var status = await manager.Ask<ReminderStatusResponse>( + new GetReminderStatusQuery(recurringReminder.Id), + TimeSpan.FromSeconds(3), + TestContext.Current.CancellationToken); + Assert.Equal(1, status.SkippedDuplicates); + Assert.Equal(ReminderManagerActor.MaxConcurrentExecutions, + (await manager.Ask<ReminderHealthResponse>( + GetReminderHealthQuery.Instance, + TimeSpan.FromSeconds(3), + TestContext.Current.CancellationToken)).ActiveExecutions); }, duration: TimeSpan.FromSeconds(5), cancellationToken: TestContext.Current.CancellationToken); + var stored = _definitionStore.Get(recurringReminder.Id); + Assert.NotNull(stored); + Assert.True(stored!.Enabled); + Assert.Equal(invocationCount, _sessionPipeline.InvocationCount); + Assert.DoesNotContain(_notificationSink.Alerts, alert => alert.Category == AlertType.ReminderExecutionFailed - && alert.Source == expiringId); + && alert.Source == recurringId); } [Fact] @@ -1207,7 +1286,7 @@ await AwaitAssertAsync(async () => } [Fact] - public async Task Second_fire_of_executing_reminder_waits_in_the_deferred_queue() + public async Task Exact_active_delivery_attempt_is_ignored() { var manager = await GetManagerAsync(); @@ -1224,20 +1303,19 @@ public async Task Second_fire_of_executing_reminder_waits_in_the_deferred_queue( var definition = CreateCurrentSessionDefinition("dup-guard-test", deliveryRequired: false); _definitionStore.Save(definition); - var envelope1 = CreateEnvelope(definition.Id.Value); - var envelope2 = CreateEnvelope(definition.Id.Value); + var envelope = CreateEnvelope(definition.Id.Value); // Both envelopes go into the actor's mailbox before either is processed, // so the second always arrives while the first execution is still in flight. - manager.Tell(envelope1); - manager.Tell(envelope2); + manager.Tell(envelope); + manager.Tell(envelope); // Exactly one delivery should reach the gateway. await deliveryProbe.ExpectMsgAsync<DeliverTrustedSessionTurn>( TimeSpan.FromSeconds(5), cancellationToken: TestContext.Current.CancellationToken); await deliveryProbe.ExpectNoMsgAsync(TimeSpan.FromMilliseconds(300), TestContext.Current.CancellationToken); - // Health reflects one active execution. The second occurrence waits. + // The active execution remains the sole settlement owner. var health = await manager.Ask<ReminderHealthResponse>( GetReminderHealthQuery.Instance, TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken); Assert.Equal(1, health.ActiveExecutions); @@ -1247,6 +1325,32 @@ await deliveryProbe.ExpectMsgAsync<DeliverTrustedSessionTurn>( a.Category == AlertType.ReminderExecutionFailed && a.Source == definition.Id.Value); } + [Fact] + public async Task Unsafe_acknowledgement_lease_does_not_start_execution() + { + var manager = await GetManagerAsync(); + var definition = CreateDefinition("unsafe-lease", "Unsafe lease"); + _definitionStore.Save(definition); + var invocationCount = _sessionPipeline.InvocationCount; + var now = TimeProvider.System.GetUtcNow(); + var envelope = new ReminderEnvelope<ReminderPayload>( + new ReminderEntity(ReminderManagerActor.ShardRegionName, ReminderManagerActor.EntityId), + new ReminderKey(definition.Id.Value), + now, + new ReminderDeadline(now.AddMinutes(60)), + new ReminderPayload { Id = definition.Id }); + var controlProbe = CreateTestProbe("unsafe-lease-control"); + + controlProbe.Send(manager, envelope); + controlProbe.Send(manager, GetReminderHealthQuery.Instance); + var health = await controlProbe.ExpectMsgAsync<ReminderHealthResponse>( + TimeSpan.FromSeconds(5), + cancellationToken: TestContext.Current.CancellationToken); + + Assert.Equal(0, health.ActiveExecutions); + Assert.Equal(invocationCount, _sessionPipeline.InvocationCount); + } + /// <summary> /// Test-only gateway stub: handles <see cref="DeliverTrustedSessionTurn"/> /// by forwarding to a probe for assertions and immediately replying diff --git a/src/Netclaw.Actors/Reminders/ActiveExecutionTracker.cs b/src/Netclaw.Actors/Reminders/ActiveExecutionTracker.cs index 78642e407..118410d93 100644 --- a/src/Netclaw.Actors/Reminders/ActiveExecutionTracker.cs +++ b/src/Netclaw.Actors/Reminders/ActiveExecutionTracker.cs @@ -23,8 +23,12 @@ internal sealed class ActiveExecutionTracker public void Add( ReminderId reminderId, Guid executionId, - ReminderEnvelope<ReminderPayload> envelope) => - _executing.Add(reminderId, new ActiveReminderExecution(executionId, envelope)); + ReminderEnvelope<ReminderPayload> envelope, + DateTimeOffset startedAt) => + _executing.Add(reminderId, new ActiveReminderExecution(executionId, envelope, startedAt)); + + public bool TryGet(ReminderId reminderId, out ActiveReminderExecution execution) => + _executing.TryGetValue(reminderId, out execution!); public bool TryRemove( ReminderId reminderId, @@ -45,4 +49,5 @@ public bool TryRemove( internal sealed record ActiveReminderExecution( Guid ExecutionId, - ReminderEnvelope<ReminderPayload> Envelope); + ReminderEnvelope<ReminderPayload> Envelope, + DateTimeOffset StartedAt); diff --git a/src/Netclaw.Actors/Reminders/ReminderExecutionActor.cs b/src/Netclaw.Actors/Reminders/ReminderExecutionActor.cs index e38987d17..f325313be 100644 --- a/src/Netclaw.Actors/Reminders/ReminderExecutionActor.cs +++ b/src/Netclaw.Actors/Reminders/ReminderExecutionActor.cs @@ -1,4 +1,4 @@ -// ----------------------------------------------------------------------- +// ----------------------------------------------------------------------- // <copyright file="ReminderExecutionActor.cs" company="Petabridge, LLC"> // Copyright (C) 2026 - 2026 Petabridge, LLC <https://petabridge.com> // </copyright> @@ -59,18 +59,14 @@ internal sealed class ReminderExecutionActor : ReceiveActor private readonly Guid _executionId; private readonly ReminderDefinition _definition; - private readonly ReminderHistoryStore _historyStore; private readonly TimeProvider _timeProvider; - private readonly ReminderEnvelope<ReminderPayload>? _envelope; + private readonly ReminderEnvelope<ReminderPayload> _envelope; private readonly ILoggingAdapter _log; private readonly DateTimeOffset _dispatchedAt; - private IReminderClient? _reminderClient; - private readonly SessionPipelineHandle _handle; private readonly ExecutionOutputAccumulator _accumulator; private bool _completed; private string? _sessionIdValue; - private HistoryRecord? _pendingHistory; private bool _awaitingDeliveryResult; private ReminderId? _expectedReminderDeliveryKey; private ICancelable? _deliveryTimeoutCancelable; @@ -84,21 +80,18 @@ public static Props CreateProps( ReminderDefinition definition, ISessionPipeline pipeline, TimeProvider timeProvider, - ReminderHistoryStore historyStore, - ReminderEnvelope<ReminderPayload>? envelope = null) => - Props.Create(() => new ReminderExecutionActor(executionId, definition, pipeline, timeProvider, historyStore, envelope)); + ReminderEnvelope<ReminderPayload> envelope) => + Props.Create(() => new ReminderExecutionActor(executionId, definition, pipeline, timeProvider, envelope)); public ReminderExecutionActor( Guid executionId, ReminderDefinition definition, ISessionPipeline pipeline, TimeProvider timeProvider, - ReminderHistoryStore historyStore, - ReminderEnvelope<ReminderPayload>? envelope = null) + ReminderEnvelope<ReminderPayload> envelope) { _executionId = executionId; _definition = definition; - _historyStore = historyStore; _timeProvider = timeProvider; _envelope = envelope; _dispatchedAt = timeProvider.GetUtcNow(); @@ -124,6 +117,7 @@ public ReminderExecutionActor( Receive<DeliveryBackstopTimeout>(HandleDeliveryBackstopTimeout); Receive<ExecutionAttemptTimeoutReached>(_ => HandleExecutionAttemptTimeout()); Receive<OutputStreamTerminated>(HandleOutputStreamTerminated); + Receive<ReminderExecutionAccepted>(HandleExecutionAccepted); Receive<ReceiveTimeout>(_ => HandleExecutionStall()); } @@ -132,13 +126,6 @@ protected override void PreStart() _log.Info( $"ReminderExecution Dispatched: execution_id={_executionId} reminder_id={_definition.Id} title={_definition.Title} schedule_type={_definition.Schedule.Type} dispatched_at={_dispatchedAt} delivery_kind={_definition.Delivery.Kind}"); - if (_envelope is not null) - { - var extension = ReminderClientExtension.Get(Context.System); - _reminderClient = extension.CreateClient( - new ReminderEntity(ReminderManagerActor.ShardRegionName, ReminderManagerActor.EntityId)); - } - _executionTimeoutCancelable = Context.System.Scheduler.ScheduleTellOnceCancelable( ExecutionAttemptTimeout, Self, @@ -155,7 +142,7 @@ private async Task InitializeAsync() { var sessionId = !string.IsNullOrWhiteSpace(_definition.Delivery.SessionId) ? new SessionId(_definition.Delivery.SessionId) - : new SessionId($"reminder/{_definition.Id}/{(_envelope?.DueTimeUtc ?? _dispatchedAt).ToUnixTimeMilliseconds()}"); + : new SessionId($"reminder/{_definition.Id}/{_envelope.DueTimeUtc.ToUnixTimeMilliseconds()}"); _sessionIdValue = sessionId.Value; var audience = _definition.Audience; @@ -210,23 +197,19 @@ await inputQueue.OfferAsync(new ChannelInput catch (Exception ex) { LogFullException(ex, "ReminderExecution InitializationFailed"); - ReportAndStop(false, ex.Message); + ReportOutcome(false, ex.Message); } } /// <summary> - /// Mode B path: dispatches the reminder as a <c>DeliverTrustedSessionTurn</c> - /// to the originating channel's gateway and calls - /// <c>IReminderClient.AckAsync(envelope)</c> exactly once once the - /// target session has acknowledged receipt via the - /// <c>MessageSource.AckTarget</c>-propagated <c>CommandAck</c>. When - /// <c>DeliveryRequired</c> is true, ack is further gated on a + /// Mode B dispatches the reminder as a <c>DeliverTrustedSessionTurn</c> + /// to the origin gateway. It reports success after the target session accepts the turn. + /// When <c>DeliveryRequired</c> is true, success also requires a /// <see cref="ReminderDeliveryResult"/> reporting an actual successful /// post (the binding actor tells it directly via /// <c>MessageSource.DeliveryObserver</c>). On <c>CommandNack</c>, a - /// delivery failure, the backstop timeout, or any exception, - /// <c>AckAsync</c> is NOT called and Akka.Reminders redelivers per its - /// built-in policy. + /// delivery failure, the backstop timeout, or an exception produces a failed outcome. + /// The reminder manager settles the Akka.Reminders occurrence. /// </summary> private async Task InitializeCurrentSessionAsync() { @@ -244,9 +227,7 @@ private async Task InitializeCurrentSessionAsync() // re-runs it (duplicate delivery). The envelope's scheduled fire time // (DueTimeUtc) is identical on every redelivery; _dispatchedAt is // captured fresh per execution actor and drifts, defeating the dedup. - // Deferred re-runs carry no envelope and are never redelivered, so the - // dispatch time is a fine fallback there. - var fireTimeMs = (_envelope?.DueTimeUtc ?? _dispatchedAt).ToUnixTimeMilliseconds(); + var fireTimeMs = _envelope.DueTimeUtc.ToUnixTimeMilliseconds(); var reminderDeliveryKey = $"{_definition.Id}:{fireTimeMs}"; _log.Info( @@ -281,7 +262,7 @@ private async Task InitializeCurrentSessionAsync() var gateway = ResolveGatewayFor(originChannelType); if (gateway is null) { - ReportAndStop(false, $"Mode B unsupported origin channel type: {originChannelType}"); + ReportOutcome(false, $"Mode B unsupported origin channel type: {originChannelType}"); return; } @@ -309,21 +290,21 @@ private async Task InitializeCurrentSessionAsync() break; } - ReportAndStop(true); + ReportOutcome(true); break; case CommandNack nack: _log.Warning( "reminder_current_session_nack execution_id={ExecutionId} reminder_id={ReminderId} session_id={SessionId} reason={Reason}", _executionId, _definition.Id, sessionId.Value, nack.Reason); - ReportAndStop(false, $"Session rejected reminder delivery: {nack.Reason}"); + ReportOutcome(false, $"Session rejected reminder delivery: {nack.Reason}"); break; default: _log.Warning( "reminder_current_session_unexpected_reply execution_id={ExecutionId} reminder_id={ReminderId} reply_type={ReplyType}", _executionId, _definition.Id, ack?.GetType().FullName ?? "null"); - ReportAndStop(false, "Unexpected reply from channel gateway"); + ReportOutcome(false, "Unexpected reply from channel gateway"); break; } } @@ -332,13 +313,13 @@ private async Task InitializeCurrentSessionAsync() _log.Warning( "reminder_current_session_timeout execution_id={ExecutionId} reminder_id={ReminderId} session_id={SessionId} timeout={Timeout}", _executionId, _definition.Id, sessionId.Value, ReminderSettings.DefaultAckTimeout); - ReportAndStop(false, "Timed out waiting for session ack"); + ReportOutcome(false, "Timed out waiting for session ack"); } } catch (Exception ex) { LogFullException(ex, "ReminderExecution CurrentSession InitializationFailed"); - ReportAndStop(false, ex.Message); + ReportOutcome(false, ex.Message); } } @@ -383,14 +364,14 @@ private void HandleDeliveryResult(ReminderDeliveryResult result) _log.Info( "reminder_delivery_observed execution_id={ExecutionId} reminder_id={ReminderId} key={ReminderDeliveryKey} channel={ChannelType}", _executionId, _definition.Id, result.ReminderDeliveryKey, result.ChannelType); - ReportAndStop(true); + ReportOutcome(true); } else { _log.Warning( "reminder_delivery_failed execution_id={ExecutionId} reminder_id={ReminderId} key={ReminderDeliveryKey} channel={ChannelType} reason={Reason}", _executionId, _definition.Id, result.ReminderDeliveryKey, result.ChannelType, result.FailureReason); - ReportAndStop(false, result.FailureReason ?? "channel reported delivery failure"); + ReportOutcome(false, result.FailureReason ?? "channel reported delivery failure"); } } @@ -407,45 +388,7 @@ private void HandleDeliveryBackstopTimeout(DeliveryBackstopTimeout msg) _log.Warning( "reminder_delivery_observation_timeout execution_id={ExecutionId} reminder_id={ReminderId} key={ReminderDeliveryKey} timeout={Timeout}", _executionId, _definition.Id, msg.ReminderDeliveryKey, DeliveryObservedTimeout); - ReportAndStop(false, $"delivery not observed within {DeliveryObservedTimeout}"); - } - - private async Task<(bool Success, string? ErrorMessage, bool OccurrenceTerminal)> SettleOccurrenceAsync( - bool executionSucceeded, - string? errorMessage) - { - if (_envelope is null) - return (executionSucceeded, errorMessage, false); - - if (executionSucceeded) - { - var ackResponse = await _reminderClient!.AckAsync(_envelope); - if (ackResponse.ResponseCode == ReminderAckResponseCode.Success) - return (true, null, false); - - var ackError = ackResponse.Message ?? $"Reminder acknowledgement returned {ackResponse.ResponseCode}."; - _log.Warning( - "reminder_ack_non_success execution_id={ExecutionId} reminder_id={ReminderId} response={ResponseCode} message={Message}", - _executionId, _definition.Id, ackResponse.ResponseCode, ackResponse.Message); - return (false, ackError, false); - } - - var reason = string.IsNullOrWhiteSpace(errorMessage) - ? "Reminder execution failed." - : errorMessage; - var nackResponse = await _reminderClient!.NackAsync(_envelope, reason); - var terminal = nackResponse.ResponseCode is ReminderNackResponseCode.Failed - or ReminderNackResponseCode.Expired; - - if (nackResponse.ResponseCode is ReminderNackResponseCode.Error - or ReminderNackResponseCode.NotFound) - { - _log.Warning( - "reminder_nack_non_success execution_id={ExecutionId} reminder_id={ReminderId} response={ResponseCode} message={Message}", - _executionId, _definition.Id, nackResponse.ResponseCode, nackResponse.Message); - } - - return (false, reason, terminal); + ReportOutcome(false, $"delivery not observed within {DeliveryObservedTimeout}"); } private IActorRef? ResolveGatewayFor(ChannelType originChannelType) @@ -559,30 +502,30 @@ private void HandleOutput(ExecutionOutput wrapper) switch (action) { case OutputAction.TurnCompleted: - { - var result = _accumulator.GetAccumulatedText(); - var notifyFailureMessage = _accumulator.BuildNotifyFailureMessage( - _definition.Delivery.Kind == DeliveryKind.Channel, - _definition.DeliveryRequired); - var success = notifyFailureMessage is null; - _log.Info( - $"ReminderExecution Completed: execution_id={_executionId} reminder_id={_definition.Id} title={_definition.Title} success={success} output_length={result.Length} notify_attempted={_accumulator.NotifyAttempted} notify_failed={_accumulator.NotifyFailed} dispatched_at={_dispatchedAt} completed_at={_timeProvider.GetUtcNow()}"); - - ReportAndStop(success, notifyFailureMessage); - break; - } + { + var result = _accumulator.GetAccumulatedText(); + var notifyFailureMessage = _accumulator.BuildNotifyFailureMessage( + _definition.Delivery.Kind == DeliveryKind.Channel, + _definition.DeliveryRequired); + var success = notifyFailureMessage is null; + _log.Info( + $"ReminderExecution Completed: execution_id={_executionId} reminder_id={_definition.Id} title={_definition.Title} success={success} output_length={result.Length} notify_attempted={_accumulator.NotifyAttempted} notify_failed={_accumulator.NotifyFailed} dispatched_at={_dispatchedAt} completed_at={_timeProvider.GetUtcNow()}"); + + ReportOutcome(success, notifyFailureMessage); + break; + } case OutputAction.Error: - { - var completedAt = _timeProvider.GetUtcNow(); - var failedMsg = $"ReminderExecution Failed: execution_id={_executionId} reminder_id={_definition.Id} title={_definition.Title} success=false error_type={_accumulator.LastErrorCategory} error_message={_accumulator.LastErrorMessage} dispatched_at={_dispatchedAt} completed_at={completedAt}"; - if (_accumulator.LastErrorCause is not null) - _log.Error(_accumulator.LastErrorCause, "{0}\n{1}", failedMsg, _accumulator.LastErrorCause.ToString()); - else - _log.Warning("{0}", failedMsg); - ReportAndStop(false, _accumulator.LastErrorMessage); - break; - } + { + var completedAt = _timeProvider.GetUtcNow(); + var failedMsg = $"ReminderExecution Failed: execution_id={_executionId} reminder_id={_definition.Id} title={_definition.Title} success=false error_type={_accumulator.LastErrorCategory} error_message={_accumulator.LastErrorMessage} dispatched_at={_dispatchedAt} completed_at={completedAt}"; + if (_accumulator.LastErrorCause is not null) + _log.Error(_accumulator.LastErrorCause, "{0}\n{1}", failedMsg, _accumulator.LastErrorCause.ToString()); + else + _log.Warning("{0}", failedMsg); + ReportOutcome(false, _accumulator.LastErrorMessage); + break; + } } } @@ -595,7 +538,7 @@ private void HandleExecutionStall() _log.Warning( "ReminderExecution Stalled: execution_id={0} reminder_id={1} title={2} no session output for {3} (elapsed={4}); concluding as failed to release the execution guard.", _executionId, _definition.Id, _definition.Title, ExecutionStallTimeout, elapsed); - ReportAndStop(false, $"Reminder execution stalled: no session output for {ExecutionStallTimeout}."); + ReportOutcome(false, $"Reminder execution stalled: no session output for {ExecutionStallTimeout}."); } private void HandleExecutionAttemptTimeout() @@ -606,7 +549,7 @@ private void HandleExecutionAttemptTimeout() _log.Warning( "ReminderExecution reached its absolute limit: execution_id={0} reminder_id={1} title={2} timeout={3}.", _executionId, _definition.Id, _definition.Title, ExecutionAttemptTimeout); - ReportAndStop(false, $"Reminder execution exceeded {ExecutionAttemptTimeout}."); + ReportOutcome(false, $"Reminder execution exceeded {ExecutionAttemptTimeout}."); } private void HandleOutputStreamTerminated(OutputStreamTerminated terminated) @@ -615,10 +558,10 @@ private void HandleOutputStreamTerminated(OutputStreamTerminated terminated) return; var reason = terminated.Failure?.Message ?? "Session output ended without a terminal result."; - ReportAndStop(false, reason); + ReportOutcome(false, reason); } - private void ReportAndStop(bool success, string? errorMessage = null) + private void ReportOutcome(bool success, string? errorMessage = null) { if (_completed || _settlementStarted) return; @@ -629,33 +572,10 @@ private void ReportAndStop(bool success, string? errorMessage = null) _executionTimeoutCancelable?.Cancel(); _executionTimeoutCancelable = null; - RunTask(async () => - { - try - { - var settlement = await SettleOccurrenceAsync(success, errorMessage); - CompleteExecution( - settlement.Success, - settlement.ErrorMessage, - settlement.OccurrenceTerminal); - } - catch (Exception ex) - { - LogFullException(ex, "ReminderExecution SettlementFailed"); - CompleteExecution(false, ex.Message, occurrenceTerminal: false); - } - - await _handle.DrainAsync(); - Context.Stop(Self); - }); - } - - private void CompleteExecution(bool success, string? errorMessage, bool occurrenceTerminal) - { _completed = true; var durationMs = (long)(_timeProvider.GetUtcNow() - _dispatchedAt).TotalMilliseconds; - _pendingHistory = new HistoryRecord( + var history = new HistoryRecord( FiredAt: _dispatchedAt, Success: success, DurationMs: durationMs, @@ -672,8 +592,20 @@ private void CompleteExecution(bool success, string? errorMessage, bool occurren _executionId, _definition.Id, success, - errorMessage, - occurrenceTerminal)); + history, + errorMessage)); + } + + private void HandleExecutionAccepted(ReminderExecutionAccepted accepted) + { + if (!_completed || accepted.ExecutionId != _executionId) + return; + + RunTask(async () => + { + await _handle.DrainAsync(); + Context.Stop(Self); + }); } protected override void PostStop() @@ -683,19 +615,6 @@ protected override void PostStop() _executionTimeoutCancelable?.Cancel(); _executionTimeoutCancelable = null; - if (_pendingHistory is not null) - { - try - { - _historyStore.AppendAsync(_definition.Id, _pendingHistory) - .GetAwaiter().GetResult(); - } - catch (Exception ex) - { - _log.Warning(ex, "Failed to write execution history for reminder '{0}'", _definition.Id); - } - } - try { _handle.Dispose(); diff --git a/src/Netclaw.Actors/Reminders/ReminderManagerActor.cs b/src/Netclaw.Actors/Reminders/ReminderManagerActor.cs index a387f4547..970d08ad7 100644 --- a/src/Netclaw.Actors/Reminders/ReminderManagerActor.cs +++ b/src/Netclaw.Actors/Reminders/ReminderManagerActor.cs @@ -1,4 +1,4 @@ -// ----------------------------------------------------------------------- +// ----------------------------------------------------------------------- // <copyright file="ReminderManagerActor.cs" company="Petabridge, LLC"> // Copyright (C) 2026 - 2026 Petabridge, LLC <https://petabridge.com> // </copyright> @@ -9,6 +9,7 @@ using Akka.Reminders; using Netclaw.Actors.Channels; using Netclaw.Configuration; +using AkkaReminderProtocol = Akka.Reminders.ReminderProtocol; using static Netclaw.Actors.Reminders.ReminderProtocol; namespace Netclaw.Actors.Reminders; @@ -40,6 +41,8 @@ public sealed partial class ReminderManagerActor : ReceiveActor /// <summary>Recent run records returned by the per-reminder status query.</summary> internal const int RecentHistoryCount = 5; + internal static readonly TimeSpan SettlementMargin = TimeSpan.FromMinutes(1); + private readonly ISessionPipeline _pipeline; private readonly EffectivePolicyDefaults _defaults; private readonly SchedulingConfig _schedulingConfig; @@ -53,7 +56,6 @@ public sealed partial class ReminderManagerActor : ReceiveActor private IReminderClient? _client; private readonly ActiveExecutionTracker _activeExecutions = new(); - private readonly Queue<DeferredReminderOccurrence> _deferredQueue = new(); private readonly Dictionary<ReminderId, int> _skipCounts = []; public ReminderManagerActor( @@ -85,7 +87,7 @@ public ReminderManagerActor( ReceiveAsync<GetReminderCommand>(HandleGetAsync); ReceiveAsync<ReminderEnvelope<ReminderPayload>>(HandleReminderFiredAsync); - ReceiveAsync<ReminderExecutionCompleted>(HandleExecutionCompletedAsync); + ReceiveAsync<ReminderExecutionCompleted>(HandleExecutionOutcomeAsync); ReceiveAsync<ReminderExecutionTerminated>(HandleExecutionTerminatedAsync); ReceiveAsync<ReconcileReminders>(_ => HandleReconcileAsync()); @@ -286,7 +288,6 @@ static ReminderSavedResponse ValidationFailure(ReminderId id, string title, stri if (exists) { await CancelScheduleOnlyAsync(id); - RemoveFromDeferredQueue(id); } DateTimeOffset? nextFire = null; @@ -323,7 +324,6 @@ static ReminderSavedResponse ValidationFailure(ReminderId id, string title, stri else { await CancelScheduleOnlyAsync(id); - RemoveFromDeferredQueue(id); } _definitionStore.Save(normalized); @@ -432,7 +432,6 @@ private async Task<ReminderStateResponse> DisableReminderInternalAsync(ReminderI await CancelScheduleOnlyAsync(id); _skipCounts.Remove(id); - RemoveFromDeferredQueue(id); _log.Info("Disabled reminder '{0}'", id.Value); return new ReminderStateResponse(id, Found: true, Enabled: false); @@ -447,7 +446,6 @@ private async Task DeleteReminderInternalAsync(ReminderId id) _definitionStore.Delete(id); await CancelScheduleOnlyAsync(id); _skipCounts.Remove(id); - RemoveFromDeferredQueue(id); try { @@ -465,7 +463,7 @@ private async Task<ReminderStateResponse> EnableReminderInternalAsync(ReminderId if (definition is null) return new ReminderStateResponse(id, Found: false, Enabled: false, ErrorMessage: "Reminder not found."); - definition = definition with + var candidate = definition with { Enabled = true, ConsecutiveFailures = 0, @@ -473,24 +471,17 @@ private async Task<ReminderStateResponse> EnableReminderInternalAsync(ReminderId UpdatedAtMs = _timeProvider.GetUtcNow().ToUnixTimeMilliseconds() }; - var scheduleResult = await ScheduleDefinitionAsync(definition, rescheduleFromNow: true); + var scheduleResult = await ScheduleDefinitionAsync(candidate, rescheduleFromNow: true); if (!scheduleResult.IsSuccess) { - definition = definition with - { - Enabled = false, - UpdatedAtMs = _timeProvider.GetUtcNow().ToUnixTimeMilliseconds() - }; - _definitionStore.Save(definition); - return new ReminderStateResponse( id, Found: true, - Enabled: false, + Enabled: definition.Enabled, ErrorMessage: scheduleResult.ErrorMessage); } - _definitionStore.Save(definition); + _definitionStore.Save(candidate); _log.Info("Enabled reminder '{0}'", id.Value); return new ReminderStateResponse( @@ -572,7 +563,6 @@ private async Task HandleReminderFiredAsync(ReminderEnvelope<ReminderPayload> en { _log.Warning("Reminder '{0}' fired while disabled. Cancelling any lingering schedule.", reminderId.Value); await CancelScheduleOnlyAsync(reminderId); - RemoveFromDeferredQueue(reminderId); await _client!.AckAsync(envelope); return; } @@ -590,13 +580,6 @@ private async Task HandleReminderFiredAsync(ReminderEnvelope<ReminderPayload> en _log.Info("Reminder fired: id='{0}', title='{1}', schedule_type={2}", reminderId.Value, definition.Title, definition.Schedule.Type); - if (_activeExecutions.IsExecuting(reminderId)) - { - RecordSkippedDuplicate(reminderId, definition.Title, "scheduled"); - EnqueueDeferredOccurrence(envelope); - return; - } - // Cron reminders are implemented as recurring single-shot schedules. if (definition.Schedule.Type == ReminderScheduleType.Cron) { @@ -607,11 +590,48 @@ private async Task HandleReminderFiredAsync(ReminderEnvelope<ReminderPayload> en } } + if (_activeExecutions.TryGet(reminderId, out var activeExecution)) + { + RecordSkippedDuplicate(reminderId, definition.Title, "active"); + if (IsSameDeliveryAttempt(activeExecution.Envelope, envelope)) + return; + + var sameOccurrence = activeExecution.Envelope.Key == envelope.Key + && activeExecution.Envelope.DueTimeUtc == envelope.DueTimeUtc; + await SettleBlockedOccurrenceAsync( + definition, + envelope, + nack: definition.Schedule.Type == ReminderScheduleType.OneShot || sameOccurrence, + "Another execution for this reminder is active."); + return; + } + if (_activeExecutions.Count >= MaxConcurrentExecutions) { - _log.Info("Concurrency limit reached ({0}), deferring reminder '{1}'", + _log.Info("Concurrency limit reached ({0}), settling blocked reminder '{1}'", MaxConcurrentExecutions, reminderId.Value); - EnqueueDeferredOccurrence(envelope); + if (definition.Schedule.Type != ReminderScheduleType.OneShot) + RecordSkippedDuplicate(reminderId, definition.Title, "capacity"); + await SettleBlockedOccurrenceAsync( + definition, + envelope, + nack: definition.Schedule.Type == ReminderScheduleType.OneShot, + "Reminder execution capacity is unavailable."); + return; + } + + if (!HasSafeExecutionLease(envelope, _timeProvider.GetUtcNow())) + { + _log.Warning( + "Reminder '{0}' does not have enough acknowledgement lease for a complete attempt.", + reminderId.Value); + if (definition.Schedule.Type != ReminderScheduleType.OneShot) + RecordSkippedDuplicate(reminderId, definition.Title, "lease"); + await SettleBlockedOccurrenceAsync( + definition, + envelope, + nack: definition.Schedule.Type == ReminderScheduleType.OneShot, + "The remaining acknowledgement lease cannot contain a complete execution attempt."); return; } @@ -620,21 +640,57 @@ private async Task HandleReminderFiredAsync(ReminderEnvelope<ReminderPayload> en StartExecution(definition, envelope); } - private void EnqueueDeferredOccurrence(ReminderEnvelope<ReminderPayload> envelope) + private static bool IsSameDeliveryAttempt( + ReminderEnvelope<ReminderPayload> active, + ReminderEnvelope<ReminderPayload> candidate) => + active.Key == candidate.Key + && active.DueTimeUtc == candidate.DueTimeUtc + && active.Deadline == candidate.Deadline; + + private static bool HasSafeExecutionLease( + ReminderEnvelope<ReminderPayload> envelope, + DateTimeOffset now) => + envelope.Deadline.IsInfinite + || envelope.Deadline.UtcDateTime - now >= ReminderExecutionActor.ExecutionAttemptTimeout + SettlementMargin; + + private async Task SettleBlockedOccurrenceAsync( + ReminderDefinition definition, + ReminderEnvelope<ReminderPayload> envelope, + bool nack, + string reason) { - if (_deferredQueue.Any(item => - item.Envelope.Key == envelope.Key - && item.Envelope.DueTimeUtc == envelope.DueTimeUtc)) + try { - return; - } + if (nack) + { + var response = await _client!.NackAsync(envelope, reason); + if (response.ResponseCode is ReminderNackResponseCode.Error or ReminderNackResponseCode.NotFound) + { + EmitSettlementFailure( + definition, + $"Negative acknowledgement returned {response.ResponseCode}: {response.Message}"); + } - _deferredQueue.Enqueue(new DeferredReminderOccurrence(envelope.Message.Id, envelope)); + return; + } + + var ack = await _client!.AckAsync(envelope); + if (ack.ResponseCode != ReminderAckResponseCode.Success) + { + EmitSettlementFailure( + definition, + $"Acknowledgement returned {ack.ResponseCode}: {ack.Message}"); + } + } + catch (Exception ex) + { + EmitSettlementFailure(definition, ex.Message, ex); + } } /// <summary> - /// Records an occurrence that waits while the same reminder runs. - /// The status command exposes this in-memory overlap count. + /// Records an occurrence that Netclaw skips or returns to Akka.Reminders. + /// The status command exposes this process-local skip count. /// </summary> private void RecordSkippedDuplicate(ReminderId reminderId, string title, string source) { @@ -668,12 +724,32 @@ private void PostFailureNoticeToChannel(ReminderDefinition? definition, string t _channelNotifier.NotifyFailure(target, text); } - private async Task HandleExecutionCompletedAsync(ReminderExecutionCompleted completed) + private async Task HandleExecutionOutcomeAsync(ReminderExecutionCompleted outcome) { - if (!_activeExecutions.TryRemove(completed.Id, completed.ExecutionId, out _)) + var replyTo = Sender; + if (!_activeExecutions.TryGet(outcome.Id, out var execution) + || execution.ExecutionId != outcome.ExecutionId) + { + replyTo.Tell(new ReminderExecutionAccepted(outcome.ExecutionId)); return; + } - await ApplyExecutionResultAsync(completed); + try + { + await SettleExecutionOutcomeAsync(outcome, execution); + } + catch (Exception ex) + { + _log.Error(ex, "Unexpected reminder settlement failure for '{0}'", outcome.Id.Value); + var definition = _definitionStore.Get(outcome.Id); + if (definition is not null) + EmitSettlementFailure(definition, ex.Message, ex); + } + finally + { + _activeExecutions.TryRemove(outcome.Id, outcome.ExecutionId, out _); + replyTo.Tell(new ReminderExecutionAccepted(outcome.ExecutionId)); + } } private async Task HandleExecutionTerminatedAsync(ReminderExecutionTerminated terminated) @@ -682,118 +758,196 @@ private async Task HandleExecutionTerminatedAsync(ReminderExecutionTerminated te return; const string reason = "Reminder execution actor terminated unexpectedly."; - var nack = await _client!.NackAsync(execution.Envelope, reason); - var terminal = nack.ResponseCode is ReminderNackResponseCode.Failed - or ReminderNackResponseCode.Expired; - - await ApplyExecutionResultAsync(new ReminderExecutionCompleted( - terminated.ExecutionId, - terminated.Id, + var now = _timeProvider.GetUtcNow(); + var definition = _definitionStore.Get(terminated.Id); + var sessionId = definition?.Delivery.Kind == DeliveryKind.CurrentSession + ? definition.Delivery.SessionId ?? $"reminder/{terminated.Id}/unknown" + : $"reminder/{terminated.Id}/{execution.Envelope.DueTimeUtc.ToUnixTimeMilliseconds()}"; + var history = new HistoryRecord( + execution.StartedAt, Success: false, - ErrorMessage: reason, - OccurrenceTerminal: terminal)); + DurationMs: (long)(now - execution.StartedAt).TotalMilliseconds, + sessionId, + reason); + + try + { + await SettleExecutionOutcomeAsync(new ReminderExecutionCompleted( + terminated.ExecutionId, + terminated.Id, + Success: false, + history, + reason), execution); + } + catch (Exception ex) + { + _log.Error(ex, "Unexpected termination settlement failed for reminder '{0}'", terminated.Id.Value); + if (definition is not null) + EmitSettlementFailure(definition, ex.Message, ex); + } } - private async Task ApplyExecutionResultAsync(ReminderExecutionCompleted completed) + private async Task SettleExecutionOutcomeAsync( + ReminderExecutionCompleted outcome, + ActiveReminderExecution execution) { - var definition = _definitionStore.Get(completed.Id); - var title = definition?.Title ?? completed.Id.Value; + await AppendHistorySafelyAsync(outcome.Id, outcome.History); - if (completed.Success) + var definition = _definitionStore.Get(outcome.Id); + if (outcome.Success) { - if (definition is not null) + await SettleSuccessfulExecutionAsync(outcome, execution, definition); + return; + } + + await SettleFailedExecutionAsync(outcome, execution, definition); + } + + private async Task AppendHistorySafelyAsync(ReminderId id, HistoryRecord history) + { + try + { + await _historyStore.AppendAsync(id, history); + } + catch (Exception ex) + { + _log.Warning(ex, "Failed to write execution history for reminder '{0}'", id.Value); + } + } + + private async Task SettleSuccessfulExecutionAsync( + ReminderExecutionCompleted outcome, + ActiveReminderExecution execution, + ReminderDefinition? definition) + { + if (definition is not null) + { + try { - definition = definition with + _definitionStore.Save(definition with { ConsecutiveFailures = 0, - Enabled = definition.Schedule.Type is ReminderScheduleType.OneShot - ? false - : definition.Enabled, - TerminalOutcome = definition.Schedule.Type is ReminderScheduleType.OneShot - ? ReminderTerminalOutcome.Completed - : null, UpdatedAtMs = _timeProvider.GetUtcNow().ToUnixTimeMilliseconds() - }; - _definitionStore.Save(definition); + }); } + catch (Exception ex) + { + EmitSettlementFailure(definition, ex.Message, ex); + return; + } + } - _log.Info("Reminder '{0}' execution completed successfully", completed.Id.Value); + AkkaReminderProtocol.ReminderAckResponse ack; + try + { + ack = await _client!.AckAsync(execution.Envelope); } - else + catch (Exception ex) + { + if (definition is not null) + EmitSettlementFailure(definition, ex.Message, ex); + return; + } + + if (ack.ResponseCode != ReminderAckResponseCode.Success) { - var count = (definition?.ConsecutiveFailures ?? 0) + 1; if (definition is not null) + { + EmitSettlementFailure( + definition, + ack.Message ?? $"Reminder acknowledgement returned {ack.ResponseCode}."); + } + + return; + } + + if (definition is { Schedule.Type: ReminderScheduleType.OneShot }) + { + try + { + _definitionStore.Save(definition with + { + Enabled = false, + ConsecutiveFailures = 0, + TerminalOutcome = ReminderTerminalOutcome.Completed, + UpdatedAtMs = _timeProvider.GetUtcNow().ToUnixTimeMilliseconds() + }); + } + catch (Exception ex) + { + _log.Error(ex, "Failed to save completed state for one-shot reminder '{0}'", outcome.Id.Value); + } + } + + _log.Info("Reminder '{0}' execution completed successfully", outcome.Id.Value); + } + + private async Task SettleFailedExecutionAsync( + ReminderExecutionCompleted outcome, + ActiveReminderExecution execution, + ReminderDefinition? definition) + { + var reason = string.IsNullOrWhiteSpace(outcome.ErrorMessage) + ? "Reminder execution failed." + : outcome.ErrorMessage; + var count = (definition?.ConsecutiveFailures ?? 0) + 1; + var thresholdReached = count >= FailurePauseThreshold; + + if (definition is not null) + { + try { definition = definition with { ConsecutiveFailures = count, + Enabled = thresholdReached ? false : definition.Enabled, + TerminalOutcome = thresholdReached ? ReminderTerminalOutcome.Failed : definition.TerminalOutcome, UpdatedAtMs = _timeProvider.GetUtcNow().ToUnixTimeMilliseconds() }; _definitionStore.Save(definition); } + catch (Exception ex) + { + EmitSettlementFailure(definition, ex.Message, ex); + return; + } + } - _log.Warning("Reminder '{0}' execution failed ({1}/{2}): {3}", - completed.Id.Value, - count, - FailurePauseThreshold, - completed.ErrorMessage); + AkkaReminderProtocol.ReminderNackResponse? nack = null; + try + { + nack = await _client!.NackAsync(execution.Envelope, reason); + } + catch (Exception ex) + { + if (definition is not null) + EmitSettlementFailure(definition, ex.Message, ex); + } - _notificationSink.Emit(OperationalAlert.Create( - _timeProvider, - "reminder.execution.failed", - AlertType.ReminderExecutionFailed, - $"Reminder '{title}' execution failed: {completed.ErrorMessage}", - AlertSeverity.Warning, - source: completed.Id.Value, - context: new Dictionary<string, string> - { - ["reminderId"] = completed.Id.Value, - ["title"] = title, - ["error"] = completed.ErrorMessage ?? "unknown", - })); - - // Surface the failure where the operator expects this reminder's - // output: its destination channel. Only below the threshold — on the - // threshold-hitting failure the disabled notice below already carries - // the last error, so posting both would double the noise on the most - // important event. Bounded overall by the threshold, never the - // unbounded skip stream that #1494 makes visible via status instead. - if (count < FailurePauseThreshold) + var occurrenceTerminal = nack?.ResponseCode is ReminderNackResponseCode.Failed + or ReminderNackResponseCode.Expired; + if (nack?.ResponseCode is ReminderNackResponseCode.Error or ReminderNackResponseCode.NotFound) + { + if (definition is not null) { - PostFailureNoticeToChannel( + EmitSettlementFailure( definition, - $"Reminder \"{title}\" failed: {completed.ErrorMessage ?? "unknown error"}"); + nack.Message ?? $"Negative acknowledgement returned {nack.ResponseCode}."); } + } - if (count >= FailurePauseThreshold || completed.OccurrenceTerminal) - { - var disableReason = count >= FailurePauseThreshold - ? $"failure threshold ({FailurePauseThreshold})" - : "the occurrence retry budget"; - _log.Warning("Reminder '{0}' hit {1}, disabling", - completed.Id.Value, - disableReason); - - _notificationSink.Emit(OperationalAlert.Create( - _timeProvider, - "reminder.auto_disabled", - AlertType.ReminderAutoDisabled, - $"Reminder '{title}' disabled after {count} consecutive failures", - AlertSeverity.Critical, - source: completed.Id.Value, - context: new Dictionary<string, string> - { - ["reminderId"] = completed.Id.Value, - ["title"] = title, - ["failureCount"] = count.ToString(), - })); - - PostFailureNoticeToChannel( - definition, - $"Reminder \"{title}\" was automatically disabled after {count} consecutive failures. " + - $"Last error: {completed.ErrorMessage ?? "unknown error"}"); + ReportExecutionFailure( + outcome.Id, + definition, + count, + reason, + willDisable: thresholdReached || occurrenceTerminal); - if (definition is not null) + if (thresholdReached || occurrenceTerminal) + { + if (!thresholdReached && definition is not null) + { + try { definition = definition with { @@ -803,13 +957,86 @@ private async Task ApplyExecutionResultAsync(ReminderExecutionCompleted complete }; _definitionStore.Save(definition); } - - await CancelScheduleOnlyAsync(completed.Id); - RemoveFromDeferredQueue(completed.Id); + catch (Exception ex) + { + EmitSettlementFailure(definition, ex.Message, ex); + } } + + await CancelScheduleOnlyAsync(outcome.Id); } + } + + private void ReportExecutionFailure( + ReminderId id, + ReminderDefinition? definition, + int count, + string reason, + bool willDisable) + { + var title = definition?.Title ?? id.Value; + _log.Warning("Reminder '{0}' execution failed ({1}/{2}): {3}", + id.Value, count, FailurePauseThreshold, reason); + + _notificationSink.Emit(OperationalAlert.Create( + _timeProvider, + "reminder.execution.failed", + AlertType.ReminderExecutionFailed, + $"Reminder '{title}' execution failed: {reason}", + AlertSeverity.Warning, + source: id.Value, + context: new Dictionary<string, string> + { + ["reminderId"] = id.Value, + ["title"] = title, + ["error"] = reason + })); - await ProcessDeferredQueueAsync(); + if (!willDisable) + { + PostFailureNoticeToChannel(definition, $"Reminder \"{title}\" failed: {reason}"); + return; + } + + _notificationSink.Emit(OperationalAlert.Create( + _timeProvider, + "reminder.auto_disabled", + AlertType.ReminderAutoDisabled, + $"Reminder '{title}' disabled after {count} consecutive failures or a terminal occurrence", + AlertSeverity.Critical, + source: id.Value, + context: new Dictionary<string, string> + { + ["reminderId"] = id.Value, + ["title"] = title, + ["failureCount"] = count.ToString() + })); + + PostFailureNoticeToChannel( + definition, + $"Reminder \"{title}\" was automatically disabled after {count} consecutive failures or a terminal occurrence. Last error: {reason}"); + } + + private void EmitSettlementFailure(ReminderDefinition definition, string reason, Exception? exception = null) + { + if (exception is null) + _log.Warning("Reminder settlement failed for '{0}': {1}", definition.Id.Value, reason); + else + _log.Error(exception, "Reminder settlement failed for '{0}': {1}", definition.Id.Value, reason); + + _notificationSink.Emit(OperationalAlert.Create( + _timeProvider, + "reminder.settlement.failed", + AlertType.ReminderExecutionFailed, + $"Reminder '{definition.Title}' settlement failed: {reason}", + AlertSeverity.Warning, + source: definition.Id.Value, + context: new Dictionary<string, string> + { + ["reminderId"] = definition.Id.Value, + ["title"] = definition.Title, + ["error"] = reason + })); } private async Task HandleReconcileAsync() @@ -925,16 +1152,16 @@ private void StartExecution( ReminderEnvelope<ReminderPayload> envelope) { var executionId = Guid.NewGuid(); - _activeExecutions.Add(definition.Id, executionId, envelope); + var startedAt = _timeProvider.GetUtcNow(); + _activeExecutions.Add(definition.Id, executionId, envelope, startedAt); - var actorName = $"exec-{SanitizeActorName(definition.Id.Value)}-{_timeProvider.GetUtcNow().ToUnixTimeMilliseconds()}"; + var actorName = $"exec-{SanitizeActorName(definition.Id.Value)}-{startedAt.ToUnixTimeMilliseconds()}"; var executionActor = Context.ActorOf( ReminderExecutionActor.CreateProps( executionId, definition, _pipeline, _timeProvider, - _historyStore, envelope), actorName); Context.WatchWith( @@ -946,59 +1173,6 @@ private void StartExecution( definition.Id, envelope.DueTimeUtc, executionActor.Path); } - private async Task ProcessDeferredQueueAsync() - { - var candidates = _deferredQueue.Count; - while (_deferredQueue.Count > 0 - && _activeExecutions.Count < MaxConcurrentExecutions - && candidates-- > 0) - { - var deferred = _deferredQueue.Dequeue(); - var nextId = deferred.Id; - var definition = _definitionStore.Get(nextId); - if (definition is null || !definition.Enabled) - { - await _client!.AckAsync(deferred.Envelope); - continue; - } - - if (_activeExecutions.IsExecuting(nextId)) - { - EnqueueDeferredOccurrence(deferred.Envelope); - continue; - } - - var now = _timeProvider.GetUtcNow(); - if (definition.Schedule.Type is not ReminderScheduleType.OneShot - && definition.ExpiresAt is { } expiresAt - && expiresAt <= now) - { - _log.Info("Deferred reminder '{0}' expired while queued (expiresAt={1}), disabling", nextId.Value, expiresAt); - await DisableReminderInternalAsync(nextId); - continue; - } - - StartExecution(definition, deferred.Envelope); - } - } - - private void RemoveFromDeferredQueue(ReminderId id) - { - if (_deferredQueue.Count == 0) - return; - - var keep = new Queue<DeferredReminderOccurrence>(); - while (_deferredQueue.Count > 0) - { - var item = _deferredQueue.Dequeue(); - if (item.Id != id) - keep.Enqueue(item); - } - - while (keep.Count > 0) - _deferredQueue.Enqueue(keep.Dequeue()); - } - private async Task<ScheduleAttempt> ScheduleDefinitionAsync(ReminderDefinition definition, bool rescheduleFromNow) { if (_client is null) @@ -1014,52 +1188,52 @@ private async Task<ScheduleAttempt> ScheduleDefinitionAsync(ReminderDefinition d switch (definition.Schedule.Type) { case ReminderScheduleType.OneShot: - { - if (definition.Schedule.FireAt is null) - return ScheduleAttempt.Fail("One-shot reminders require an absolute fire time."); + { + if (definition.Schedule.FireAt is null) + return ScheduleAttempt.Fail("One-shot reminders require an absolute fire time."); - var fireAt = definition.Schedule.FireAt.Value; - if (fireAt <= now) - return ScheduleAttempt.Fail("One-shot fire time is in the past."); + var fireAt = definition.Schedule.FireAt.Value; + if (fireAt <= now) + return ScheduleAttempt.Fail("One-shot fire time is in the past."); - var result = await _client.ScheduleSingleReminderAsync(key, fireAt, payload); - return result.ResponseCode == ReminderScheduleResponseCode.Success - ? ScheduleAttempt.Ok(fireAt) - : ScheduleAttempt.Fail(result.Message ?? "Failed to schedule one-shot reminder."); - } + var result = await _client.ScheduleSingleReminderAsync(key, fireAt, payload); + return result.ResponseCode == ReminderScheduleResponseCode.Success + ? ScheduleAttempt.Ok(fireAt) + : ScheduleAttempt.Fail(result.Message ?? "Failed to schedule one-shot reminder."); + } case ReminderScheduleType.Interval: - { - if (definition.Schedule.Interval is null) - return ScheduleAttempt.Fail("Interval reminders require an interval duration."); - - var interval = definition.Schedule.Interval.Value; - var first = rescheduleFromNow - ? now.Add(interval) - : definition.Schedule.FireAt is { } explicitFirst && explicitFirst > now - ? explicitFirst - : now.Add(interval); - - var result = await _client.ScheduleRecurringReminderAsync(key, first, interval, payload); - return result.ResponseCode == ReminderScheduleResponseCode.Success - ? ScheduleAttempt.Ok(first) - : ScheduleAttempt.Fail(result.Message ?? "Failed to schedule interval reminder."); - } + { + if (definition.Schedule.Interval is null) + return ScheduleAttempt.Fail("Interval reminders require an interval duration."); + + var interval = definition.Schedule.Interval.Value; + var first = rescheduleFromNow + ? now.Add(interval) + : definition.Schedule.FireAt is { } explicitFirst && explicitFirst > now + ? explicitFirst + : now.Add(interval); + + var result = await _client.ScheduleRecurringReminderAsync(key, first, interval, payload); + return result.ResponseCode == ReminderScheduleResponseCode.Success + ? ScheduleAttempt.Ok(first) + : ScheduleAttempt.Fail(result.Message ?? "Failed to schedule interval reminder."); + } case ReminderScheduleType.Cron: - { - if (string.IsNullOrWhiteSpace(definition.Schedule.CronExpression)) - return ScheduleAttempt.Fail("Cron reminders require a cron expression."); + { + if (string.IsNullOrWhiteSpace(definition.Schedule.CronExpression)) + return ScheduleAttempt.Fail("Cron reminders require a cron expression."); - var nextFire = CronScheduleHelper.GetNextOccurrence(definition.Schedule.CronExpression, _timeProvider); - if (nextFire is null) - return ScheduleAttempt.Fail("Cron schedule has no future occurrence."); + var nextFire = CronScheduleHelper.GetNextOccurrence(definition.Schedule.CronExpression, _timeProvider); + if (nextFire is null) + return ScheduleAttempt.Fail("Cron schedule has no future occurrence."); - var result = await _client.ScheduleSingleReminderAsync(key, nextFire.Value, payload); - return result.ResponseCode == ReminderScheduleResponseCode.Success - ? ScheduleAttempt.Ok(nextFire) - : ScheduleAttempt.Fail(result.Message ?? "Failed to schedule cron reminder."); - } + var result = await _client.ScheduleSingleReminderAsync(key, nextFire.Value, payload); + return result.ResponseCode == ReminderScheduleResponseCode.Success + ? ScheduleAttempt.Ok(nextFire) + : ScheduleAttempt.Fail(result.Message ?? "Failed to schedule cron reminder."); + } default: return ScheduleAttempt.Fail("Unknown schedule type."); @@ -1238,10 +1412,6 @@ private sealed record ScheduleAttempt(bool IsSuccess, DateTimeOffset? NextFire, public static ScheduleAttempt Fail(string message) => new(false, null, message); } - private sealed record DeferredReminderOccurrence( - ReminderId Id, - ReminderEnvelope<ReminderPayload> Envelope) : INoSerializationVerificationNeeded; - private sealed record ReminderAudienceAuthorizationResult(bool IsSuccess, TrustAudience? EffectiveAudience, string? ErrorMessage) : INoSerializationVerificationNeeded { public static ReminderAudienceAuthorizationResult Success(TrustAudience effectiveAudience) diff --git a/src/Netclaw.Actors/Reminders/ReminderProtocol.cs b/src/Netclaw.Actors/Reminders/ReminderProtocol.cs index 9a4179a32..c948c8ab1 100644 --- a/src/Netclaw.Actors/Reminders/ReminderProtocol.cs +++ b/src/Netclaw.Actors/Reminders/ReminderProtocol.cs @@ -302,149 +302,149 @@ public enum ReminderSaveError public static partial class ReminderProtocol { -/// <summary>Marker for reminder commands.</summary> -public interface IReminderCommand; + /// <summary>Marker for reminder commands.</summary> + public interface IReminderCommand; -/// <summary>Marker for reminder queries.</summary> -public interface IReminderQuery; + /// <summary>Marker for reminder queries.</summary> + public interface IReminderQuery; -/// <summary>Marker for reminder responses.</summary> -public interface IReminderResponse; + /// <summary>Marker for reminder responses.</summary> + public interface IReminderResponse; -// ===== Commands ===== + // ===== Commands ===== -public sealed record SaveReminderCommand( - ReminderDefinition Definition, - ReminderWriteMode WriteMode = ReminderWriteMode.CreateOnly, - ReminderAudienceAuthorizationContext? Authorization = null) : IReminderCommand, INoSerializationVerificationNeeded; + public sealed record SaveReminderCommand( + ReminderDefinition Definition, + ReminderWriteMode WriteMode = ReminderWriteMode.CreateOnly, + ReminderAudienceAuthorizationContext? Authorization = null) : IReminderCommand, INoSerializationVerificationNeeded; -public sealed record ReminderAudienceAuthorizationContext( - TrustAudience? SourceAudience, - string? SourceDescription = null) : INoSerializationVerificationNeeded; + public sealed record ReminderAudienceAuthorizationContext( + TrustAudience? SourceAudience, + string? SourceDescription = null) : INoSerializationVerificationNeeded; -/// <summary> -/// Disables a reminder and cancels any active schedule. The definition file -/// is preserved on disk so history and configuration remain available for diagnosis. -/// </summary> -public sealed record CancelReminderCommand(ReminderId Id) : IReminderCommand, INoSerializationVerificationNeeded; + /// <summary> + /// Disables a reminder and cancels any active schedule. The definition file + /// is preserved on disk so history and configuration remain available for diagnosis. + /// </summary> + public sealed record CancelReminderCommand(ReminderId Id) : IReminderCommand, INoSerializationVerificationNeeded; -/// <summary> -/// Permanently deletes a reminder definition, its schedule, and history from disk. -/// Not exposed as an LLM tool — use via CLI (<c>netclaw reminder delete</c>) or HTTP API. -/// </summary> -public sealed record DeleteReminderCommand(ReminderId Id) : IReminderCommand, INoSerializationVerificationNeeded; -public sealed record DisableReminderCommand(ReminderId Id) : IReminderCommand, INoSerializationVerificationNeeded; -public sealed record EnableReminderCommand(ReminderId Id) : IReminderCommand, INoSerializationVerificationNeeded; -public sealed record ListRemindersCommand(bool IncludeDisabled = true) : IReminderQuery, INoSerializationVerificationNeeded; + /// <summary> + /// Permanently deletes a reminder definition, its schedule, and history from disk. + /// Not exposed as an LLM tool — use via CLI (<c>netclaw reminder delete</c>) or HTTP API. + /// </summary> + public sealed record DeleteReminderCommand(ReminderId Id) : IReminderCommand, INoSerializationVerificationNeeded; + public sealed record DisableReminderCommand(ReminderId Id) : IReminderCommand, INoSerializationVerificationNeeded; + public sealed record EnableReminderCommand(ReminderId Id) : IReminderCommand, INoSerializationVerificationNeeded; + public sealed record ListRemindersCommand(bool IncludeDisabled = true) : IReminderQuery, INoSerializationVerificationNeeded; -// ===== Queries ===== + // ===== Queries ===== -public sealed record GetReminderCommand(ReminderId Id) : IReminderQuery, INoSerializationVerificationNeeded; + public sealed record GetReminderCommand(ReminderId Id) : IReminderQuery, INoSerializationVerificationNeeded; -// ===== Responses ===== + // ===== Responses ===== -public sealed record ReminderSavedResponse( - ReminderId Id, - string Title, - bool Success, - DateTimeOffset? NextFire, - ReminderSaveError Error = ReminderSaveError.None, - string? ErrorMessage = null) : IReminderResponse, INoSerializationVerificationNeeded; + public sealed record ReminderSavedResponse( + ReminderId Id, + string Title, + bool Success, + DateTimeOffset? NextFire, + ReminderSaveError Error = ReminderSaveError.None, + string? ErrorMessage = null) : IReminderResponse, INoSerializationVerificationNeeded; -public sealed record ReminderCancelledResponse(ReminderId Id, bool Found) : IReminderResponse, INoSerializationVerificationNeeded; -public sealed record ReminderDeletedResponse(ReminderId Id, bool Found) : IReminderResponse, INoSerializationVerificationNeeded; + public sealed record ReminderCancelledResponse(ReminderId Id, bool Found) : IReminderResponse, INoSerializationVerificationNeeded; + public sealed record ReminderDeletedResponse(ReminderId Id, bool Found) : IReminderResponse, INoSerializationVerificationNeeded; -public sealed record ReminderStateResponse( - ReminderId Id, - bool Found, - bool Enabled, - DateTimeOffset? NextFire = null, - string? ErrorMessage = null) : IReminderResponse, INoSerializationVerificationNeeded; + public sealed record ReminderStateResponse( + ReminderId Id, + bool Found, + bool Enabled, + DateTimeOffset? NextFire = null, + string? ErrorMessage = null) : IReminderResponse, INoSerializationVerificationNeeded; -public sealed record ReminderListResponse(IReadOnlyList<ReminderInfo> Reminders) : IReminderResponse, INoSerializationVerificationNeeded; -public sealed record GetReminderResponse(ReminderInfo? Reminder) : IReminderResponse, INoSerializationVerificationNeeded; + public sealed record ReminderListResponse(IReadOnlyList<ReminderInfo> Reminders) : IReminderResponse, INoSerializationVerificationNeeded; + public sealed record GetReminderResponse(ReminderInfo? Reminder) : IReminderResponse, INoSerializationVerificationNeeded; -// ===== Delivery / Health ===== + // ===== Delivery / Health ===== -/// <summary> -/// Point-to-point delivery outcome sent by a channel binding actor directly -/// back to the dispatching <see cref="ReminderExecutionActor"/> (carried as -/// <see cref="Channels.MessageSource.DeliveryObserver"/> on the originating -/// <c>DeliverTrustedSessionTurn</c>) when a reminder-sourced turn completes. -/// Used for <see cref="DeliveryKind.CurrentSession"/> with -/// <see cref="ReminderDefinition.DeliveryRequired"/> = true to gate envelope -/// ack on whether the assistant reply actually reached the channel. -/// <para> -/// Unlike the prior EventStream-broadcast observation, this signal reports -/// <see cref="Delivered"/> = false on a failed post, so the execution actor -/// can report failure immediately (triggering Akka.Reminders redelivery) -/// instead of waiting out the backstop timeout. -/// </para> -/// </summary> -/// <param name="ReminderDeliveryKey"> -/// Composite key in format "{reminderId}:{fireTimestampMs}". -/// </param> -/// <param name="ChannelType"> -/// The channel that attempted the delivery. -/// </param> -/// <param name="Delivered"> -/// True when the assistant reply was posted to the channel; false when the -/// turn completed without a successful post. -/// </param> -/// <param name="FailureReason"> -/// Optional human-readable reason when <see cref="Delivered"/> is false. -/// </param> -/// <param name="ObservedAtMs"> -/// Optional timestamp when the outbound delivery outcome was observed. -/// </param> -public sealed record ReminderDeliveryResult( - ReminderId ReminderDeliveryKey, - Channels.ChannelType ChannelType, - bool Delivered, - string? FailureReason = null, - long? ObservedAtMs = null) : IReminderResponse, INoSerializationVerificationNeeded; - -// ===== Health query ===== + /// <summary> + /// Point-to-point delivery outcome sent by a channel binding actor directly + /// back to the dispatching <see cref="ReminderExecutionActor"/> (carried as + /// <see cref="Channels.MessageSource.DeliveryObserver"/> on the originating + /// <c>DeliverTrustedSessionTurn</c>) when a reminder-sourced turn completes. + /// Used for <see cref="DeliveryKind.CurrentSession"/> with + /// <see cref="ReminderDefinition.DeliveryRequired"/> = true to gate envelope + /// ack on whether the assistant reply actually reached the channel. + /// <para> + /// Unlike the prior EventStream-broadcast observation, this signal reports + /// <see cref="Delivered"/> = false on a failed post, so the execution actor + /// can report failure immediately (triggering Akka.Reminders redelivery) + /// instead of waiting out the backstop timeout. + /// </para> + /// </summary> + /// <param name="ReminderDeliveryKey"> + /// Composite key in format "{reminderId}:{fireTimestampMs}". + /// </param> + /// <param name="ChannelType"> + /// The channel that attempted the delivery. + /// </param> + /// <param name="Delivered"> + /// True when the assistant reply was posted to the channel; false when the + /// turn completed without a successful post. + /// </param> + /// <param name="FailureReason"> + /// Optional human-readable reason when <see cref="Delivered"/> is false. + /// </param> + /// <param name="ObservedAtMs"> + /// Optional timestamp when the outbound delivery outcome was observed. + /// </param> + public sealed record ReminderDeliveryResult( + ReminderId ReminderDeliveryKey, + Channels.ChannelType ChannelType, + bool Delivered, + string? FailureReason = null, + long? ObservedAtMs = null) : IReminderResponse, INoSerializationVerificationNeeded; + + // ===== Health query ===== -/// <summary> -/// Query sent to <see cref="ReminderManagerActor"/> to obtain current health counters. -/// </summary> -public sealed record GetReminderHealthQuery : IReminderQuery, INoSerializationVerificationNeeded -{ - public static readonly GetReminderHealthQuery Instance = new(); -} + /// <summary> + /// Query sent to <see cref="ReminderManagerActor"/> to obtain current health counters. + /// </summary> + public sealed record GetReminderHealthQuery : IReminderQuery, INoSerializationVerificationNeeded + { + public static readonly GetReminderHealthQuery Instance = new(); + } -/// <summary> -/// Response from <see cref="GetReminderHealthQuery"/> with current runtime counters. -/// </summary> -public sealed record ReminderHealthResponse( - int ScheduledCount, - int ActiveExecutions, - int FailedCount) : IReminderResponse, INoSerializationVerificationNeeded; + /// <summary> + /// Response from <see cref="GetReminderHealthQuery"/> with current runtime counters. + /// </summary> + public sealed record ReminderHealthResponse( + int ScheduledCount, + int ActiveExecutions, + int FailedCount) : IReminderResponse, INoSerializationVerificationNeeded; -/// <summary> -/// Query sent to <see cref="ReminderManagerActor"/> for the per-reminder -/// operational status surfaced by <c>netclaw reminder status <id></c>. -/// </summary> -public sealed record GetReminderStatusQuery(ReminderId Id) : IReminderQuery, INoSerializationVerificationNeeded; + /// <summary> + /// Query sent to <see cref="ReminderManagerActor"/> for the per-reminder + /// operational status surfaced by <c>netclaw reminder status <id></c>. + /// </summary> + public sealed record GetReminderStatusQuery(ReminderId Id) : IReminderQuery, INoSerializationVerificationNeeded; -/// <summary> -/// Response to <see cref="GetReminderStatusQuery"/>: per-reminder health for an -/// operator — whether the reminder exists/is enabled, whether an execution is in -/// flight right now, when it next fires, the durable failure count, the -/// in-memory overlap count, and recent run history. -/// </summary> -public sealed record ReminderStatusResponse( - ReminderId Id, - bool Found, - bool Enabled, - bool Executing, - DateTimeOffset? NextFire, - int ConsecutiveFailures, - int SkippedDuplicates, - ReminderTerminalOutcome? TerminalOutcome, - ReminderOccurrenceInfo? Occurrence, - IReadOnlyList<HistoryRecord> RecentHistory) : IReminderResponse, INoSerializationVerificationNeeded; + /// <summary> + /// Response to <see cref="GetReminderStatusQuery"/>: per-reminder health for an + /// operator — whether the reminder exists/is enabled, whether an execution is in + /// flight right now, when it next fires, the durable failure count, the + /// process-local skipped occurrence count, and recent run history. + /// </summary> + public sealed record ReminderStatusResponse( + ReminderId Id, + bool Found, + bool Enabled, + bool Executing, + DateTimeOffset? NextFire, + int ConsecutiveFailures, + int SkippedDuplicates, + ReminderTerminalOutcome? TerminalOutcome, + ReminderOccurrenceInfo? Occurrence, + IReadOnlyList<HistoryRecord> RecentHistory) : IReminderResponse, INoSerializationVerificationNeeded; } @@ -473,8 +473,10 @@ internal sealed record ReminderExecutionCompleted( Guid ExecutionId, ReminderId Id, bool Success, - string? ErrorMessage = null, - bool OccurrenceTerminal = false) : INoSerializationVerificationNeeded; + HistoryRecord History, + string? ErrorMessage = null) : INoSerializationVerificationNeeded; + +internal sealed record ReminderExecutionAccepted(Guid ExecutionId) : INoSerializationVerificationNeeded; internal sealed record ReminderExecutionTerminated( Guid ExecutionId, diff --git a/src/Netclaw.Cli/Reminder/ReminderCommand.cs b/src/Netclaw.Cli/Reminder/ReminderCommand.cs index dbcaa4c23..3cbf1c63a 100644 --- a/src/Netclaw.Cli/Reminder/ReminderCommand.cs +++ b/src/Netclaw.Cli/Reminder/ReminderCommand.cs @@ -1,4 +1,4 @@ -// ----------------------------------------------------------------------- +// ----------------------------------------------------------------------- // <copyright file="ReminderCommand.cs" company="Petabridge, LLC"> // Copyright (C) 2026 - 2026 Petabridge, LLC <https://petabridge.com> // </copyright> @@ -592,7 +592,7 @@ private static async Task<int> RunStatusAsync(DaemonApi api, string[] args) Console.WriteLine($"Executing now: {status.Executing}"); Console.WriteLine($"Next fire: {status.NextFire ?? "not scheduled"}"); Console.WriteLine($"Consecutive fails: {status.ConsecutiveFailures}"); - Console.WriteLine($"Deferred overlaps: {status.SkippedDuplicates}"); + Console.WriteLine($"Skipped occurrences: {status.SkippedDuplicates}"); Console.WriteLine($"Terminal outcome: {status.TerminalOutcome ?? "none"}"); if (status.Occurrence is { } occurrence) diff --git a/src/Netclaw.Daemon.Tests/Reminder/ReminderEndpointAuthorizationTests.cs b/src/Netclaw.Daemon.Tests/Reminder/ReminderEndpointAuthorizationTests.cs index e30f8d6ca..897d0438b 100644 --- a/src/Netclaw.Daemon.Tests/Reminder/ReminderEndpointAuthorizationTests.cs +++ b/src/Netclaw.Daemon.Tests/Reminder/ReminderEndpointAuthorizationTests.cs @@ -129,6 +129,56 @@ public async Task Unauthenticated_POST_reminders_returns_401() Assert.Empty(_testActor.ReceivedMessages); } + [Fact] + public async Task Operator_GET_status_returns_retry_and_terminal_fields() + { + var dueTime = _timeProvider.GetUtcNow().AddMinutes(-5); + _testActor.StatusResponse = new ReminderStatusResponse( + new ReminderId("status-fields"), + Found: true, + Enabled: false, + Executing: false, + NextFire: null, + ConsecutiveFailures: 5, + SkippedDuplicates: 2, + TerminalOutcome: ReminderTerminalOutcome.Failed, + Occurrence: new ReminderOccurrenceInfo( + dueTime, + NextAttemptAtUtc: null, + AttemptCount: 5, + LastFailureReason: "persistence recovery failed", + CompletionStatus: "Failed", + DeliveryDeadlineUtc: null, + AckDeadlineUtc: null, + CompletedAtUtc: _timeProvider.GetUtcNow()), + RecentHistory: + [ + new HistoryRecord( + dueTime, + Success: false, + DurationMs: 30000, + SessionId: "reminder/status-fields/1", + ErrorMessage: "persistence recovery failed") + ]); + await using var app = await CreateAppAsync(spoofLoopback: true); + + var response = await app.GetTestClient().GetAsync( + "/api/reminders/status-fields/status", + TestContext.Current.CancellationToken); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var json = await response.Content.ReadFromJsonAsync<JsonElement>( + TestContext.Current.CancellationToken); + Assert.False(json.GetProperty("enabled").GetBoolean()); + Assert.Equal(5, json.GetProperty("consecutiveFailures").GetInt32()); + Assert.Equal(2, json.GetProperty("skippedDuplicates").GetInt32()); + Assert.Equal("Failed", json.GetProperty("terminalOutcome").GetString()); + var occurrence = json.GetProperty("occurrence"); + Assert.Equal(5, occurrence.GetProperty("attemptCount").GetInt32()); + Assert.Equal("persistence recovery failed", occurrence.GetProperty("lastFailureReason").GetString()); + Assert.Single(json.GetProperty("recentHistory").EnumerateArray()); + } + // ── Test case 4: POST with invalid audience value → 400, no command dispatched ── [Fact] @@ -471,6 +521,7 @@ private sealed class TestReminderActor { private readonly List<object> _received = []; public IReadOnlyList<object> ReceivedMessages => _received; + public ReminderStatusResponse? StatusResponse { get; set; } public void Record(object message) => _received.Add(message); } @@ -544,6 +595,22 @@ public RecordingReminderActor(TestReminderActor sink) sink.Record(cmd); Sender.Tell(new ReminderStateResponse(cmd.Id, Found: false, Enabled: false)); }); + + Receive<GetReminderStatusQuery>(query => + { + sink.Record(query); + Sender.Tell(sink.StatusResponse ?? new ReminderStatusResponse( + query.Id, + Found: false, + Enabled: false, + Executing: false, + NextFire: null, + ConsecutiveFailures: 0, + SkippedDuplicates: 0, + TerminalOutcome: null, + Occurrence: null, + RecentHistory: [])); + }); } } From ea764a04969121342817e2c911073a2453ae5c8a Mon Sep 17 00:00:00 2001 From: Aaron Stannard <aaron@petabridge.com> Date: Sat, 8 Aug 2026 14:17:59 +0000 Subject: [PATCH 3/4] refactor(sessions): use channel input for executions --- .../SessionPipelineUnobservedTaskTests.cs | 12 +- .../Reminders/ReminderExecutionActorTests.cs | 25 +++- .../Channels/ChannelPipeline.cs | 6 +- .../Channels/SessionPipelineHandle.cs | 111 ++---------------- .../Channels/StreamTaskObservation.cs | 19 +-- .../Reminders/ReminderExecutionActor.cs | 8 +- .../Webhooks/WebhookExecutionActor.cs | 26 ++-- 7 files changed, 60 insertions(+), 147 deletions(-) diff --git a/src/Netclaw.Actors.Tests/Channels/SessionPipelineUnobservedTaskTests.cs b/src/Netclaw.Actors.Tests/Channels/SessionPipelineUnobservedTaskTests.cs index 201ad162e..54cb642e2 100644 --- a/src/Netclaw.Actors.Tests/Channels/SessionPipelineUnobservedTaskTests.cs +++ b/src/Netclaw.Actors.Tests/Channels/SessionPipelineUnobservedTaskTests.cs @@ -16,10 +16,8 @@ namespace Netclaw.Actors.Tests.Channels; /// unobserved-task crashes (daemon-unobserved logs with /// <c>AbruptTerminationException</c> / <c>StreamDetachedException</c>). /// Akka.Streams stages create internal <see cref="Task{Done}"/> instances -/// (e.g. via <c>Sink.ForEach</c>'s <c>IgnoreSink</c> and -/// <c>Source.Queue</c>'s <c>_completion</c>) that fault on teardown. -/// Production code in <c>SessionPipelineHandle</c> and -/// <c>ChannelPipeline</c> uses two complementary patterns to observe these: +/// that can fault on teardown. Production code observes these tasks before it +/// discards their materialized values. These tests cover two valid patterns: /// <list type="bullet"> /// <item><c>Keep.Both</c> + await both materialized tasks.</item> /// <item><c>MapMaterializedValue</c> with a <c>ContinueWith</c> that @@ -36,8 +34,7 @@ protected override void ConfigureAkka(Akka.Hosting.AkkaConfigurationBuilder buil /// Verifies that wrapping <c>Sink.ForEach</c> with <c>MapMaterializedValue</c> /// + <c>ContinueWith(OnlyOnFaulted)</c> reliably runs the observation /// callback when the upstream is aborted. This is the pattern used in - /// <c>ChannelPipeline.CreateAsync</c> and - /// <c>SessionPipelineHandle.InitializeWithQueueAsync</c>. + /// <c>ChannelPipeline.CreateAsync</c> and <c>SessionPipelineHandle</c>. /// </summary> [Fact] public async Task MapMaterializedValue_continuation_observes_sink_task_on_fault() @@ -74,8 +71,7 @@ public async Task MapMaterializedValue_continuation_observes_sink_task_on_fault( /// <summary> /// Verifies that <c>Keep.Both</c> + await-both observes the Sink.ForEach - /// task after a stream fault. This is the pattern used in - /// <c>SessionPipelineHandle.InitializeWithChannelAsync</c>. + /// task after a stream fault when a caller retains both materialized tasks. /// </summary> [Fact] public async Task KeepBoth_awaits_observe_both_watch_and_sink_tasks_on_fault() diff --git a/src/Netclaw.Actors.Tests/Reminders/ReminderExecutionActorTests.cs b/src/Netclaw.Actors.Tests/Reminders/ReminderExecutionActorTests.cs index e5af89e72..a4ccccbab 100644 --- a/src/Netclaw.Actors.Tests/Reminders/ReminderExecutionActorTests.cs +++ b/src/Netclaw.Actors.Tests/Reminders/ReminderExecutionActorTests.cs @@ -330,6 +330,9 @@ public async Task Execution_fails_when_output_stream_ends_without_terminal_outpu var completed = await probe.ExpectMsgAsync<ReminderExecutionCompleted>( TimeSpan.FromSeconds(5), cancellationToken: TestContext.Current.CancellationToken); + await pipeline.InputCompleted.WaitAsync( + TimeSpan.FromSeconds(5), + TestContext.Current.CancellationToken); Assert.False(completed.Success); Assert.Contains("ended", completed.ErrorMessage!, StringComparison.OrdinalIgnoreCase); @@ -462,9 +465,12 @@ private sealed class ScriptedSessionPipeline( { private readonly TaskCompletionSource<ChannelInput> _inputCaptured = new(TaskCreationOptions.RunContinuationsAsynchronously); + private readonly TaskCompletionSource _inputCompleted = + new(TaskCreationOptions.RunContinuationsAsynchronously); public SessionPipelineOptions? CapturedOptions { get; private set; } public Task<ChannelInput> InputCaptured => _inputCaptured.Task; + public Task InputCompleted => _inputCompleted.Task; public Task<MaterializedSession> CreateAsync( SessionId sessionId, @@ -480,7 +486,11 @@ public Task<MaterializedSession> CreateAsync( { _inputCaptured.TrySetResult(ci); }) - .MapMaterializedValue<NotUsed>(_ => NotUsed.Instance); + .MapMaterializedValue<NotUsed>(completion => + { + _ = ObserveInputCompletionAsync(completion); + return NotUsed.Instance; + }); var outputs = outputFactory(sessionId).ToList(); Source<SessionOutput, NotUsed> output = Source.UnfoldAsync<int, SessionOutput>(0, async state => @@ -503,6 +513,19 @@ public Task<MaterializedSession> CreateAsync( } return Task.FromResult(new MaterializedSession(captureInputSink, output, killSwitch)); + + async Task ObserveInputCompletionAsync(Task<Done> completion) + { + try + { + await completion.ConfigureAwait(false); + _inputCompleted.TrySetResult(); + } + catch (Exception ex) + { + _inputCompleted.TrySetException(ex); + } + } } public Task SendFeedbackAsync(IWithSessionId feedback, CancellationToken ct = default) => diff --git a/src/Netclaw.Actors/Channels/ChannelPipeline.cs b/src/Netclaw.Actors/Channels/ChannelPipeline.cs index 29a9ae0ed..6ace6cd28 100644 --- a/src/Netclaw.Actors/Channels/ChannelPipeline.cs +++ b/src/Netclaw.Actors/Channels/ChannelPipeline.cs @@ -1,4 +1,4 @@ -// ----------------------------------------------------------------------- +// ----------------------------------------------------------------------- // <copyright file="ChannelPipeline.cs" company="Petabridge, LLC"> // Copyright (C) 2026 - 2026 Petabridge, LLC <https://petabridge.com> // </copyright> @@ -103,9 +103,9 @@ internal MaterializedSession( /// <summary> /// Input sink. Encapsulates <see cref="ChannelInput"/> → /// <see cref="SendUserMessage"/> transformation and delivery to the - /// session manager. Channel connects its own Source: + /// session manager. A caller connects its own source: /// <code> - /// Source.Queue<ChannelInput>(16, Backpressure) + /// Source.Channel<ChannelInput>(512, true) /// .ToMat(session.Input, Keep.Left) /// .Run(system); /// </code> diff --git a/src/Netclaw.Actors/Channels/SessionPipelineHandle.cs b/src/Netclaw.Actors/Channels/SessionPipelineHandle.cs index 96b7ff8b6..3e1cc3b13 100644 --- a/src/Netclaw.Actors/Channels/SessionPipelineHandle.cs +++ b/src/Netclaw.Actors/Channels/SessionPipelineHandle.cs @@ -17,11 +17,8 @@ namespace Netclaw.Actors.Channels; /// <summary> /// Manages the lifecycle of a materialized session pipeline on behalf of an /// owning actor. Not thread-safe — designed for use within a single actor context -/// (no concurrent access). Supports two modes: -/// <list type="bullet"> -/// <item>Long-lived (with reinitialization) for binding actors (Slack, SignalR)</item> -/// <item>Short-lived (fire-and-forget) for execution actors (Reminders, Webhooks)</item> -/// </list> +/// (no concurrent access). Callers keep the input channel open for multiple turns +/// or complete it after one turn. Long-lived callers can reinitialize the pipeline. /// The handle does not own the <see cref="ActorMaterializer"/>. The owning actor /// creates the materializer from its context and passes it in; Akka disposes it /// automatically when the actor stops. @@ -40,7 +37,7 @@ public sealed class SessionPipelineHandle private int _pipelineGeneration; private bool _isReinitializing; - // Stored from first InitializeWithChannelAsync for reinit + // Stored from the first initialization for optional reinitialization. private IActorContext? _storedContext; private SessionId? _storedSessionId; private SessionPipelineOptions? _storedOptions; @@ -60,16 +57,17 @@ public SessionPipelineHandle( /// <summary>The current pipeline generation, for <c>OutputStreamTerminated</c> filtering.</summary> public int Generation => _pipelineGeneration; - /// <summary>The <see cref="System.Threading.Channels.ChannelWriter{T}"/> for long-lived actors to write input. - /// Null if not initialized or if initialized via queue mode.</summary> + /// <summary>The <see cref="System.Threading.Channels.ChannelWriter{T}"/> for actors to write input. + /// Null if the pipeline is not initialized.</summary> public ChannelWriter<ChannelInput>? InputQueue => _inputQueue; /// <summary>Whether the handle has been initialized (session is not null).</summary> public bool IsInitialized => _session is not null; /// <summary> - /// Idempotent pipeline creation for long-lived actors that use <see cref="Source.Channel{T}(int, bool)"/> - /// for ongoing input. Stores all parameters for use by <see cref="ReinitializeAsync"/>. + /// Idempotent pipeline creation with <see cref="Source.Channel{T}(int, bool)"/> input. + /// The caller controls the input lifetime. This method stores its parameters for + /// optional use by <see cref="ReinitializeAsync"/>. /// </summary> public async Task<ChannelWriter<ChannelInput>> InitializeWithChannelAsync( IActorContext context, @@ -147,99 +145,6 @@ async Task ObserveTerminationAsync() return inputQueue; } - /// <summary> - /// Pipeline creation for fire-and-forget execution actors that use - /// <see cref="Source.Queue{T}(int,OverflowStrategy)"/>, offer input once, and complete. - /// This compatibility overload keeps the prior stream lifecycle behavior. - /// </summary> - public Task<ISourceQueueWithComplete<ChannelInput>> InitializeWithQueueAsync( - IActorContext context, - SessionId sessionId, - SessionPipelineOptions options, - Action<SessionOutput> onOutput, - CancellationToken cancellationToken = default) => - InitializeWithQueueAsync( - context, - sessionId, - options, - onOutput, - _ => { }, - cancellationToken); - - /// <summary> - /// Pipeline creation for fire-and-forget execution actors that use - /// <see cref="Source.Queue{T}(int,OverflowStrategy)"/>, offer input once, and complete. - /// Reports stream termination so the owner can fail a session that ends - /// without a terminal output. - /// </summary> - public async Task<ISourceQueueWithComplete<ChannelInput>> InitializeWithQueueAsync( - IActorContext context, - SessionId sessionId, - SessionPipelineOptions options, - Action<SessionOutput> onOutput, - Action<Exception?> onStreamTerminated, - CancellationToken cancellationToken = default) - { - _log.Info("Initializing {0} execution pipeline", _materializerNamePrefix); - - var materializer = context.Materializer(namePrefix: _materializerNamePrefix); - - var materialized = await _pipeline.CreateAsync( - sessionId, options, - materializer: materializer, - cancellationToken: cancellationToken); - - var inputQueue = Source.Queue<ChannelInput>(8, OverflowStrategy.Backpressure) - .ToMaterialized(materialized.Input, Keep.Left) - .Run(materializer); - - // SourceQueueLogic.PostStop sets StreamDetachedException on its - // _completion TCS unconditionally — observe it so unused queue-mode - // sessions don't leak the fault on teardown. - StreamTaskObservation.ObserveSilently(inputQueue.WatchCompletionAsync()); - - var outputTerminated = materialized.Output - .WatchTermination((_, done) => done) - .ToMaterialized( - Sink.ForEach<SessionOutput>(onOutput).ObservingFault(), - Keep.Left) - .Run(materializer); - - _outputCompletion = outputTerminated; - - _ = ObserveTerminationAsync(); - - _session = materialized; - - _log.Info("{0} execution pipeline initialized", _materializerNamePrefix); - return inputQueue; - - async Task ObserveTerminationAsync() - { - Exception? failure = null; - try - { - await outputTerminated.ConfigureAwait(false); - } - catch (Exception ex) - { - failure = ex; - } - - try - { - onStreamTerminated(failure); - } - catch (Exception callbackException) - { - _log.Error( - callbackException, - "{0} onStreamTerminated callback threw", - _materializerNamePrefix); - } - } - } - /// <summary> /// Tears down the current pipeline and re-initializes using the parameters /// stored from the original <see cref="InitializeWithChannelAsync"/> call. diff --git a/src/Netclaw.Actors/Channels/StreamTaskObservation.cs b/src/Netclaw.Actors/Channels/StreamTaskObservation.cs index 09732f311..f88c3216a 100644 --- a/src/Netclaw.Actors/Channels/StreamTaskObservation.cs +++ b/src/Netclaw.Actors/Channels/StreamTaskObservation.cs @@ -9,20 +9,13 @@ namespace Netclaw.Actors.Channels; /// <summary> -/// Helpers for observing the internal <see cref="Task{Done}"/> instances -/// that Akka.Streams stages (e.g. <c>Sink.ForEach</c>'s <c>IgnoreSink</c> -/// and <c>Source.Queue</c>'s <c>_completion</c>) create via -/// <c>TaskCompletionSource</c>. Those tasks are faulted on stream -/// teardown; if nothing observes them, the finalizer surfaces the fault -/// as <see cref="TaskScheduler.UnobservedTaskException"/>. Real failures -/// still surface through <c>WatchTermination</c> / actor messages — this -/// only silences the duplicate finalizer noise. +/// Observes discarded Akka.Streams sink tasks. Stream teardown can fault these +/// tasks after the owner receives the primary termination signal. /// </summary> internal static class StreamTaskObservation { /// <summary> - /// Attach a fault-only continuation that reads <see cref="Task.Exception"/> - /// so the task is marked observed before the finalizer runs. + /// Attach a fault-only continuation that reads <see cref="Task.Exception"/>. /// </summary> public static void ObserveSilently(Task task) { @@ -34,11 +27,7 @@ public static void ObserveSilently(Task task) } /// <summary> - /// Replace a sink's <see cref="Task{Done}"/> materialized value with - /// <see cref="NotUsed"/> after attaching <see cref="ObserveSilently"/> - /// to the underlying task. Use when the caller wants to discard the - /// materialized value but the underlying TCS still gets faulted on - /// teardown. + /// Replace a sink task with <see cref="NotUsed"/> after the observer attaches. /// </summary> public static Sink<TIn, NotUsed> ObservingFault<TIn>(this Sink<TIn, Task<Done>> sink) => sink.MapMaterializedValue<NotUsed>(static task => diff --git a/src/Netclaw.Actors/Reminders/ReminderExecutionActor.cs b/src/Netclaw.Actors/Reminders/ReminderExecutionActor.cs index f325313be..3caadb5e9 100644 --- a/src/Netclaw.Actors/Reminders/ReminderExecutionActor.cs +++ b/src/Netclaw.Actors/Reminders/ReminderExecutionActor.cs @@ -152,7 +152,7 @@ private async Task InitializeAsync() $"ReminderExecution Initialized: execution_id={_executionId} reminder_id={_definition.Id} session_id={sessionId.Value} audience={audience} source=stored-definition"); var self = Self; - var inputQueue = await _handle.InitializeWithQueueAsync( + var inputWriter = await _handle.InitializeWithChannelAsync( Context, sessionId, new SessionPipelineOptions @@ -161,11 +161,11 @@ private async Task InitializeAsync() Filter = OutputFilter.TextStreaming | OutputFilter.ToolCalls }, output => self.Tell(new ExecutionOutput(output)), - failure => self.Tell(new OutputStreamTerminated(failure))); + (_, failure) => self.Tell(new OutputStreamTerminated(failure))); var prompt = BuildPrompt(_definition); - await inputQueue.OfferAsync(new ChannelInput + await inputWriter.WriteAsync(new ChannelInput { SenderId = new Protocol.SenderId("reminder-system"), ChannelId = _definition.Delivery.Address, @@ -185,7 +185,7 @@ await inputQueue.OfferAsync(new ChannelInput : null }); - inputQueue.Complete(); + inputWriter.Complete(); // Arm the Mode A stall backstop: the pipeline now streams output to // this actor, each ExecutionOutput resets the ReceiveTimeout, and a diff --git a/src/Netclaw.Daemon/Webhooks/WebhookExecutionActor.cs b/src/Netclaw.Daemon/Webhooks/WebhookExecutionActor.cs index d7b4978e9..82631385a 100644 --- a/src/Netclaw.Daemon/Webhooks/WebhookExecutionActor.cs +++ b/src/Netclaw.Daemon/Webhooks/WebhookExecutionActor.cs @@ -1,4 +1,4 @@ -// ----------------------------------------------------------------------- +// ----------------------------------------------------------------------- // <copyright file="WebhookExecutionActor.cs" company="Petabridge, LLC"> // Copyright (C) 2026 - 2026 Petabridge, LLC <https://petabridge.com> // </copyright> @@ -75,7 +75,7 @@ private async Task InitializeAsync() { var self = Self; var routeAudience = _invocation.Route.Config.Audience; - var inputQueue = await _handle.InitializeWithQueueAsync( + var inputWriter = await _handle.InitializeWithChannelAsync( Context, _invocation.SessionId, new SessionPipelineOptions @@ -85,9 +85,9 @@ private async Task InitializeAsync() PromptOverlay = _invocation.Route.BuildPromptOverlay() }, output => self.Tell(new ExecutionOutput(output)), - failure => self.Tell(new OutputStreamTerminated(failure))); + (_, failure) => self.Tell(new OutputStreamTerminated(failure))); - await inputQueue.OfferAsync(new ChannelInput + await inputWriter.WriteAsync(new ChannelInput { SenderId = new SenderId($"webhook:{_invocation.Route.Name}"), ChannelId = _invocation.Route.Name, @@ -107,7 +107,7 @@ await inputQueue.OfferAsync(new ChannelInput RequestedDeliveryTarget = _invocation.Route.BuildNotificationDeliveryTarget() }); - inputQueue.Complete(); + inputWriter.Complete(); } catch (Exception ex) { @@ -125,14 +125,14 @@ private void HandleOutput(ExecutionOutput wrapper) switch (action) { case OutputAction.TurnCompleted: - { - var hasNotify = !string.IsNullOrWhiteSpace(_invocation.Route.BuildDefaultNotifyInstructions()) - || !string.IsNullOrWhiteSpace(_invocation.Route.Config.NotifyInstructions); - var deliveryRequired = _invocation.Route.Config.DeliveryRequired; - var failureMsg = _accumulator.BuildNotifyFailureMessage(hasNotify, deliveryRequired); - ReportAndStop(failureMsg is null, failureMsg); - break; - } + { + var hasNotify = !string.IsNullOrWhiteSpace(_invocation.Route.BuildDefaultNotifyInstructions()) + || !string.IsNullOrWhiteSpace(_invocation.Route.Config.NotifyInstructions); + var deliveryRequired = _invocation.Route.Config.DeliveryRequired; + var failureMsg = _accumulator.BuildNotifyFailureMessage(hasNotify, deliveryRequired); + ReportAndStop(failureMsg is null, failureMsg); + break; + } case OutputAction.Error: ReportAndStop(false, _accumulator.LastErrorMessage); break; From b6d909f07d915be3f77ca3ae2340038f695da246 Mon Sep 17 00:00:00 2001 From: Aaron Stannard <aaron@petabridge.com> Date: Sat, 8 Aug 2026 15:33:02 +0000 Subject: [PATCH 4/4] fix(reminders): clarify delayed occurrence logs --- .../Reminders/ReminderManagerActor.cs | 22 ++++++++++++++----- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/src/Netclaw.Actors/Reminders/ReminderManagerActor.cs b/src/Netclaw.Actors/Reminders/ReminderManagerActor.cs index 970d08ad7..3af97195b 100644 --- a/src/Netclaw.Actors/Reminders/ReminderManagerActor.cs +++ b/src/Netclaw.Actors/Reminders/ReminderManagerActor.cs @@ -622,16 +622,26 @@ await SettleBlockedOccurrenceAsync( if (!HasSafeExecutionLease(envelope, _timeProvider.GetUtcNow())) { - _log.Warning( - "Reminder '{0}' does not have enough acknowledgement lease for a complete attempt.", - reminderId.Value); - if (definition.Schedule.Type != ReminderScheduleType.OneShot) + var isOneShot = definition.Schedule.Type == ReminderScheduleType.OneShot; + if (isOneShot) + { + _log.Warning( + "Reminder '{0}' arrived too late to finish before its delivery deadline. Returning it to the scheduler for retry.", + reminderId.Value); + } + else + { + _log.Warning( + "Recurring reminder '{0}' arrived too late to finish before its delivery deadline. Skipping this occurrence.", + reminderId.Value); RecordSkippedDuplicate(reminderId, definition.Title, "lease"); + } + await SettleBlockedOccurrenceAsync( definition, envelope, - nack: definition.Schedule.Type == ReminderScheduleType.OneShot, - "The remaining acknowledgement lease cannot contain a complete execution attempt."); + nack: isOneShot, + "The reminder arrived too late to finish before its delivery deadline."); return; }