From 15b96a23952df96ff4d656660d7ddc7d37cd1e78 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Tue, 30 Jun 2026 16:59:44 +0000 Subject: [PATCH 1/4] feat(reminders): support operator-triggered runs --- .../PRD-008-scheduling-and-periodic-tasks.md | 4 +- .../.system/files/netclaw-operations/SKILL.md | 2 + .../references/scheduling.md | 13 + .../manual-reminder-execution/.openspec.yaml | 2 + .../manual-reminder-execution/design.md | 111 ++++++ .../manual-reminder-execution/proposal.md | 79 ++++ .../specs/netclaw-cli/spec.md | 33 ++ .../specs/netclaw-scheduling/spec.md | 96 +++++ .../specs/reminder-execution-history/spec.md | 50 +++ .../manual-reminder-execution/tasks.md | 35 ++ .../Reminders/ReminderExecutionActorTests.cs | 3 +- .../Reminders/ReminderHistoryStoreTests.cs | 36 +- .../Reminders/ReminderManagerActorTests.cs | 352 +++++++++++++++++- .../Reminders/ActiveExecutionTracker.cs | 9 +- .../Reminders/ReminderExecutionActor.cs | 31 +- .../Reminders/ReminderManagerActor.cs | 128 ++++++- .../Reminders/ReminderProtocol.cs | 29 +- .../Reminder/ReminderCommandTests.cs | 107 ++++++ src/Netclaw.Cli/Daemon/DaemonApi.cs | 7 + src/Netclaw.Cli/Reminder/ReminderCommand.cs | 54 ++- .../ReminderEndpointAuthorizationTests.cs | 73 ++++ .../ReminderEndpointRouteBuilderExtensions.cs | 38 ++ 22 files changed, 1255 insertions(+), 37 deletions(-) create mode 100644 openspec/changes/manual-reminder-execution/.openspec.yaml create mode 100644 openspec/changes/manual-reminder-execution/design.md create mode 100644 openspec/changes/manual-reminder-execution/proposal.md create mode 100644 openspec/changes/manual-reminder-execution/specs/netclaw-cli/spec.md create mode 100644 openspec/changes/manual-reminder-execution/specs/netclaw-scheduling/spec.md create mode 100644 openspec/changes/manual-reminder-execution/specs/reminder-execution-history/spec.md create mode 100644 openspec/changes/manual-reminder-execution/tasks.md create mode 100644 src/Netclaw.Cli.Tests/Reminder/ReminderCommandTests.cs diff --git a/docs/prd/PRD-008-scheduling-and-periodic-tasks.md b/docs/prd/PRD-008-scheduling-and-periodic-tasks.md index 4e5d1fee0..778cf68a2 100644 --- a/docs/prd/PRD-008-scheduling-and-periodic-tasks.md +++ b/docs/prd/PRD-008-scheduling-and-periodic-tasks.md @@ -133,6 +133,7 @@ The agent and CLI SHALL support: - Pause/resume individual tasks - Delete tasks - Show execution history for a task +- Trigger an existing task to run immediately via CLI for testing and debugging ### SCHED-006 Tool Grants @@ -156,7 +157,6 @@ Deferred to Phase 2. - Event triggers (fire on channel message matching regex) - Webhook triggers (fire on incoming POST) -- Manual triggers (fire via CLI only) - One-shot scheduled tasks (fire once at a specific time) - Per-routine persistent state (JSON blob per task) - Heartbeat system @@ -171,6 +171,8 @@ Deferred to Phase 2. 6. Tasks with ungrantable tools are rejected at creation. 7. Consecutive failures pause the task and notify the operator. 8. Max concurrent execution limit is enforced. +9. CLI can trigger an enabled scheduled task to run immediately without changing + its existing schedule. ## Cross-References diff --git a/feeds/skills/.system/files/netclaw-operations/SKILL.md b/feeds/skills/.system/files/netclaw-operations/SKILL.md index a514dbd60..b280af28c 100644 --- a/feeds/skills/.system/files/netclaw-operations/SKILL.md +++ b/feeds/skills/.system/files/netclaw-operations/SKILL.md @@ -51,6 +51,8 @@ that fires unattended cannot answer approval prompts, so pre-approve any shell v it needs first with `netclaw approvals trust-verb `. Background shell: set `_background: true` on `shell_execute` (max 5 concurrent; cancel servers/watchers when done; background jobs are killed when the session passivates). +Operators can run an enabled reminder immediately with `netclaw reminder run `; +this requires the daemon and does not change the reminder's schedule. Full detail — delivery contract, proactive channel messaging, approval scoping, job lifecycle — is in diff --git a/feeds/skills/.system/files/netclaw-operations/references/scheduling.md b/feeds/skills/.system/files/netclaw-operations/references/scheduling.md index 097c95d05..3a012b1c8 100644 --- a/feeds/skills/.system/files/netclaw-operations/references/scheduling.md +++ b/feeds/skills/.system/files/netclaw-operations/references/scheduling.md @@ -54,6 +54,19 @@ netclaw reminder cancel # disable, keep definition netclaw reminder delete # permanent delete + history ``` +To run an enabled reminder immediately without changing its schedule, use: + +``` +netclaw reminder run +``` + +Manual runs go through the same execution pipeline and write normal history with +`source=manual`, but they do not reschedule cron reminders, consume one-shot +occurrences, or count toward scheduled consecutive-failure auto-disable. The +daemon rejects manual runs when scheduling is disabled, the reminder is missing +or disabled, the recurring reminder has expired, the same reminder is already +executing, or the global reminder execution limit is full. + 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. diff --git a/openspec/changes/manual-reminder-execution/.openspec.yaml b/openspec/changes/manual-reminder-execution/.openspec.yaml new file mode 100644 index 000000000..c0d3374be --- /dev/null +++ b/openspec/changes/manual-reminder-execution/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-06-28 diff --git a/openspec/changes/manual-reminder-execution/design.md b/openspec/changes/manual-reminder-execution/design.md new file mode 100644 index 000000000..54d66fb04 --- /dev/null +++ b/openspec/changes/manual-reminder-execution/design.md @@ -0,0 +1,111 @@ +## Context + +Reminder executions are currently driven by Akka.Reminders envelopes delivered to +`ReminderManagerActor`. The manager resolves the persisted `ReminderDefinition`, +applies runtime gates, starts `ReminderExecutionActor`, and uses envelope ack / redelivery +semantics for scheduled `CurrentSession` deliveries. + +Issue #1511 needs a debugging path that executes the same persisted reminder now +without waiting for the next scheduled fire. The path must be fast for operators +but must not disturb the scheduler state, bypass stored trust context, or make +manual diagnostic failures count as scheduled production failures. + +## Goals / Non-Goals + +**Goals:** + +- Add an operator CLI/API path to run an existing enabled reminder immediately. +- Reuse the existing reminder execution actor and persisted reminder definition. +- Keep durable schedules and `nextFire` unchanged. +- Preserve scheduled envelope ack/redelivery behavior for real scheduler fires. +- Distinguish manual and scheduled runs in manager accounting and history. + +**Non-Goals:** + +- No `--allow-disabled` behavior in this change. +- No idempotency key or retry de-duplication for repeated CLI/API calls. +- No arbitrary prompt override or one-off unsaved reminder execution. +- No dashboard, Slack slash command, or TUI surface. + +## Decisions + +### D1. Manual runs enter through an explicit manager command + +Add `RunReminderNowCommand(ReminderId)` to the external reminder actor protocol. +The daemon endpoint sends this command to `ReminderManagerActor`; the manager +loads the persisted definition and starts execution directly. + +**Rationale:** This keeps the control-plane request inside the actor that already +owns duplicate execution, concurrency, expiry, and definition lookup. It avoids +touching Akka.Reminders storage for a diagnostic action. + +**Alternative rejected:** create a temporary Akka.Reminders schedule or fake +`ReminderEnvelope`. A temporary schedule can collide with the durable reminder +key or alter scheduler state; a fake envelope would make ack/redelivery behavior +lie about scheduler-owned state. + +### D2. Execution origin is first-class + +Add an execution source (`scheduled` or `manual`) to the execution start path, +internal completion message, and history record. Scheduled fires use +`scheduled`; CLI/API run-now uses `manual`. + +**Rationale:** The manager must know whether completion should update scheduled +failure counters and one-shot lifecycle. History also needs this signal so +operators do not mistake a diagnostic run for a scheduled fire. + +### D3. Manual runs fail fast rather than queueing + +When the same reminder is active or the global concurrency limit is full, manual +run requests are rejected with a structured busy response. They are not added to +the scheduled deferred queue. + +**Rationale:** The CLI command is a debugging action; a later queued run gives +operators less certainty and can mask the state they were trying to inspect. +Scheduled fires keep the existing deferred/duplicate semantics. + +### D4. Manual completion does not change scheduled failure accounting + +Manual successes and failures are written to history but do not increment, reset, +or trip the scheduled consecutive-failure counter. Manual one-shot completion does +not disable the reminder. + +**Rationale:** Diagnostic attempts should not auto-disable production reminders or +erase production failure state. Operators can inspect the manual history entry to +confirm a fix without mutating scheduled accounting. + +### D5. REST endpoint uses existing daemon auth plus Operator classification + +`POST /api/reminders/{id}/run` is placed under the existing authorized reminder +route group and rejects callers that do not map to `PrincipalClassification.Operator`. + +**Rationale:** Running a reminder can invoke tools and send messages under stored +trust context. Triggering it is therefore an operator control-plane action, not a +general authenticated-user action. + +## Risks / Trade-offs + +- **HTTP retry can duplicate a manual run after a fast completion** -> Accept for + MVP; add explicit run idempotency later if operators need scripted retries. +- **A scheduled fire arriving during a manual run is skipped by the existing + duplicate guard** -> Documented trade-off: non-overlap is safer than parallel + reminder execution. Status exposes skipped scheduled fires. +- **History files with old records lack source** -> Default missing source to + `scheduled` during read and add a regression test. +- **Manual run of CurrentSession has no scheduler redelivery** -> Intentional; + failures surface through the manual response/history path and can be retried by + the operator. + +## Migration Plan + +No config or reminder definition migration is required. Existing history JSONL +records remain readable and default to `scheduled` source when the field is +absent. Rollback leaves new records with an extra `source` JSON field; older code +that ignores unknown JSON properties can continue reading the other fields. + +## Open Questions + +- Should a follow-up add `--allow-disabled` for operators who intentionally want + to test a paused reminder without re-enabling it? +- Should scripted/API users get an optional idempotency key after the first CLI + workflow proves useful? diff --git a/openspec/changes/manual-reminder-execution/proposal.md b/openspec/changes/manual-reminder-execution/proposal.md new file mode 100644 index 000000000..676f362cc --- /dev/null +++ b/openspec/changes/manual-reminder-execution/proposal.md @@ -0,0 +1,79 @@ +## Why + +Operators debugging reminders currently have to wait for the next scheduled fire +to verify permission, delivery, and execution fixes. Issue #1511 asks for a +daemon-backed way to run an existing reminder immediately so reminder DX issues +can be diagnosed with the same speed as webhook payload replay. + +## Source PRDs + +- `PRD-008` (Scheduling and Periodic Tasks): scheduled task management, + execution history, and failure handling. +- `PRD-004` (CLI Onboarding and Configuration): operator-facing CLI management + surfaces that talk to the daemon. +- `PRD-002` (Gateway Security Envelope): default-deny, operator-authorized + control-plane actions, and fail-closed execution behavior. + +## What Changes + +- Add an operator-only immediate execution path for existing reminders: + `netclaw reminder run ` backed by `POST /api/reminders/{id}/run`. +- The daemon asks `ReminderManagerActor` to run the persisted reminder definition + through the normal `ReminderExecutionActor` pipeline without creating a durable + Akka.Reminders schedule and without synthesizing a fake scheduler envelope. +- Manual execution leaves the reminder's existing schedule untouched: no + `nextFire` mutation, no cron reschedule, and no one-shot consumption. +- Manual runs respect runtime safety gates: scheduling disabled, missing + reminder, disabled reminder, expired recurring reminder, duplicate in-flight + execution, and global concurrency exhaustion all fail loudly. +- Execution records carry whether the run was scheduled or manual so operators + can interpret history and status output correctly. + +In scope for MVP: + +- CLI, REST API, actor protocol, execution origin tracking, history output, and + tests for success/failure gates. +- Manual runs of enabled reminders only. + +Out of scope for MVP: + +- Running disabled reminders via `--allow-disabled`. +- Idempotency keys for retried HTTP/manual requests. +- Replaying arbitrary historical reminder payloads or overriding the persisted + reminder definition at run time. +- A dashboard or Slack slash command for manual execution. + +## Capabilities + +### New Capabilities + +None. + +### Modified Capabilities + +- `netclaw-scheduling`: immediate manual execution semantics, safety gates, and + scheduled-vs-manual execution accounting. +- `netclaw-cli`: add `netclaw reminder run ` as an operator command that + requires a running daemon. +- `reminder-execution-history`: execution records include the run source so + manual diagnostic runs are distinguishable from scheduler fires. + +## Impact + +- **Actor protocol:** add an external `RunReminderNowCommand` and response; add + execution origin to the internal completion message. +- **Runtime:** `ReminderManagerActor` starts manual executions directly through + `ReminderExecutionActor` with no Akka.Reminders envelope. Scheduled execution + paths continue to use existing envelope ack/redelivery behavior. +- **REST API:** add `POST /api/reminders/{id}/run`, protected by the existing + daemon authorization policy and restricted to Operator authority. +- **CLI:** add `netclaw reminder run ` and help text; daemon offline and + non-success responses produce clear errors. +- **Persistence/history:** append `source` to new history records while retaining + compatibility with existing JSONL records that lack the field. +- **Security:** no new tool grants or bypasses. Manual execution uses the stored + reminder audience/boundary and the same delivery/tool policy as scheduled + execution; the control-plane trigger is operator-only. +- **Operations:** operators can test permission and delivery changes immediately; + manual failures are visible in history but do not count toward scheduled + auto-disable thresholds. diff --git a/openspec/changes/manual-reminder-execution/specs/netclaw-cli/spec.md b/openspec/changes/manual-reminder-execution/specs/netclaw-cli/spec.md new file mode 100644 index 000000000..a1333538c --- /dev/null +++ b/openspec/changes/manual-reminder-execution/specs/netclaw-cli/spec.md @@ -0,0 +1,33 @@ +## ADDED Requirements + +### Requirement: CLI command for immediate reminder execution + +The CLI SHALL provide a `netclaw reminder run ` subcommand that asks the +running daemon to trigger immediate execution of an existing reminder. The +subcommand SHALL require a daemon connection, SHALL call the daemon reminder run +endpoint, and SHALL print a clear success or failure message. + +The command SHALL NOT edit reminder files directly and SHALL NOT attempt to run a +reminder when the daemon is offline. + +#### Scenario: Operator runs reminder immediately + +- **GIVEN** the daemon is running +- **WHEN** the operator runs `netclaw reminder run daily-summary` +- **THEN** the CLI calls `POST /api/reminders/daily-summary/run` +- **AND** exits with code `0` when the daemon accepts the run +- **AND** prints a message naming the reminder and indicating the run started + +#### Scenario: Daemon-required failure is clear + +- **GIVEN** no daemon API is available to the CLI +- **WHEN** the operator runs `netclaw reminder run daily-summary` +- **THEN** the CLI exits non-zero +- **AND** prints that `reminder run` requires a running daemon + +#### Scenario: Daemon rejection is surfaced + +- **GIVEN** the daemon rejects immediate execution because the reminder is disabled, busy, missing, or scheduling is disabled +- **WHEN** the operator runs `netclaw reminder run daily-summary` +- **THEN** the CLI exits non-zero +- **AND** prints the daemon-provided reason without claiming the run started diff --git a/openspec/changes/manual-reminder-execution/specs/netclaw-scheduling/spec.md b/openspec/changes/manual-reminder-execution/specs/netclaw-scheduling/spec.md new file mode 100644 index 000000000..e83de8a31 --- /dev/null +++ b/openspec/changes/manual-reminder-execution/specs/netclaw-scheduling/spec.md @@ -0,0 +1,96 @@ +## ADDED Requirements + +### Requirement: Operator-triggered immediate reminder execution + +The reminder manager SHALL support an operator-triggered immediate execution of +an existing enabled reminder. Immediate execution SHALL run the persisted +`ReminderDefinition` through the normal reminder execution pipeline and SHALL be +marked with execution source `manual`. + +Immediate execution SHALL NOT create, mutate, or cancel an Akka.Reminders +schedule. It SHALL NOT synthesize a fake scheduler envelope. It SHALL NOT mutate +the reminder's next scheduled fire time, reschedule cron reminders, or consume a +one-shot reminder's scheduled occurrence. + +If a one-shot scheduled occurrence fires while a manual execution for the same +reminder is already active, the manager SHALL queue exactly one scheduled +execution behind the active manual run, acknowledge the scheduler envelope, and +run the queued scheduled occurrence after the manual run completes. That queued +scheduled execution SHALL retain source `scheduled` and SHALL apply normal +one-shot completion behavior. + +The manager SHALL reject immediate execution before dispatch when scheduling is +disabled, the reminder does not exist, the reminder is disabled, the reminder is +an expired recurring reminder, the same reminder is already executing, or the +global reminder execution concurrency limit is full. Rejected immediate +executions SHALL return a structured response with a clear reason and SHALL NOT +append execution history. + +Manual execution completion SHALL append normal execution history, but manual +failure and success SHALL NOT modify the scheduled consecutive-failure counter or +trigger scheduled auto-disable behavior. + +#### Scenario: Manual execution starts without changing the schedule + +- **GIVEN** an enabled interval reminder has a scheduled next fire time +- **WHEN** an operator triggers immediate execution for that reminder +- **THEN** the manager starts a `ReminderExecutionActor` using the persisted reminder definition +- **AND** the execution source is `manual` +- **AND** no Akka.Reminders schedule is created, cancelled, or rescheduled for the manual run +- **AND** the reminder's next scheduled fire time remains unchanged + +#### Scenario: Manual execution of one-shot does not consume the reminder + +- **GIVEN** an enabled one-shot reminder has a future scheduled fire time +- **WHEN** an operator-triggered immediate execution completes successfully +- **THEN** the reminder definition remains enabled +- **AND** the original one-shot schedule remains available for its future scheduled fire + +#### Scenario: Scheduled one-shot fires while manual execution is active + +- **GIVEN** an enabled one-shot reminder is executing from an operator-triggered manual run +- **WHEN** the reminder's scheduled one-shot occurrence fires +- **THEN** the manager acknowledges the scheduler envelope +- **AND** queues one scheduled execution behind the active manual run +- **AND** does not dispatch the scheduled occurrence concurrently with the manual run +- **WHEN** the manual run completes +- **THEN** the queued scheduled execution starts with source `scheduled` +- **AND** the one-shot reminder is disabled after the scheduled execution completes + +#### Scenario: Manual execution rejects disabled reminder + +- **GIVEN** a disabled reminder definition exists +- **WHEN** an operator triggers immediate execution for that reminder +- **THEN** the manager rejects the request before dispatch +- **AND** no execution actor is started +- **AND** no execution history is appended + +#### Scenario: Manual execution rejects duplicate in-flight reminder + +- **GIVEN** a reminder is already executing from a scheduled or manual run +- **WHEN** an operator triggers immediate execution for the same reminder +- **THEN** the manager rejects the request as already executing +- **AND** the request is not queued +- **AND** the skipped-duplicate counter for scheduled fires is not incremented + +#### Scenario: Manual execution rejects when global concurrency is full + +- **GIVEN** `MaxConcurrentExecutions` reminder executions are already active +- **WHEN** an operator triggers immediate execution for another enabled reminder +- **THEN** the manager rejects the request as busy +- **AND** the request is not queued in the scheduled deferred queue + +#### Scenario: Manual execution does not affect scheduled failure accounting + +- **GIVEN** a reminder has scheduled consecutive-failure count greater than zero +- **WHEN** a manual execution for that reminder succeeds or fails +- **THEN** the scheduled consecutive-failure count remains unchanged +- **AND** manual failure does not auto-disable the reminder + +#### Scenario: CurrentSession manual execution does not use scheduler ack or redelivery + +- **GIVEN** a reminder persisted with `Delivery.Kind = CurrentSession` +- **WHEN** an operator triggers immediate execution +- **THEN** the execution actor dispatches the trusted session turn using the persisted delivery target +- **AND** no Akka.Reminders envelope is passed to the execution actor +- **AND** no scheduler ack or scheduler redelivery is attempted for the manual run diff --git a/openspec/changes/manual-reminder-execution/specs/reminder-execution-history/spec.md b/openspec/changes/manual-reminder-execution/specs/reminder-execution-history/spec.md new file mode 100644 index 000000000..cb60b7b82 --- /dev/null +++ b/openspec/changes/manual-reminder-execution/specs/reminder-execution-history/spec.md @@ -0,0 +1,50 @@ +## MODIFIED Requirements + +### Requirement: Execution record written per run + +On completion of each reminder execution (success or failure), the system SHALL append one structured record to +`~/.netclaw/reminders/{reminderId}.history.jsonl`. The record SHALL contain: +`firedAt` (ISO 8601 UTC), `success` (bool), `durationMs` (int), +`sessionId` (string, format `reminder/{id}/{firedAtMs}` for isolated sessions), +`errorMessage` (string or null), and `source` (`scheduled` or `manual`). A write +failure SHALL be logged as a warning and SHALL NOT affect the execution result +reported to the reminder manager. + +History readers SHALL treat records without a `source` field as `scheduled` for +backward compatibility with history files written before manual execution +support. + +#### Scenario: Successful scheduled execution recorded + +- **WHEN** a scheduled reminder execution completes successfully +- **THEN** a record with `success: true`, `source: "scheduled"`, elapsed `durationMs`, and the + session ID is appended to `{reminderId}.history.jsonl` +- **AND** the execution result reported to the manager is unaffected by the + write + +#### Scenario: Failed scheduled execution recorded + +- **WHEN** a scheduled reminder execution fails or times out +- **THEN** a record with `success: false`, `source: "scheduled"`, and a non-null `errorMessage` is + appended to `{reminderId}.history.jsonl` + +#### Scenario: Manual execution recorded + +- **WHEN** an operator-triggered manual reminder execution completes successfully or fails +- **THEN** a record with `source: "manual"` is appended to `{reminderId}.history.jsonl` +- **AND** the record includes the same `firedAt`, `success`, `durationMs`, `sessionId`, and `errorMessage` fields as scheduled executions + +#### Scenario: Legacy history record defaults to scheduled source + +- **GIVEN** an existing history file contains a record without a `source` field +- **WHEN** reminder history is read +- **THEN** the record is returned with source `scheduled` +- **AND** the missing field does not make the whole history read fail + +#### Scenario: History write failure does not block manager + +- **WHEN** the history file write fails (e.g., disk full, permission error) +- **THEN** a warning is logged with the reminder ID and error detail +- **AND** the completion or failure message is still sent to the manager +- **AND** the reminder's failure counter is not incremented due to the + history write failure diff --git a/openspec/changes/manual-reminder-execution/tasks.md b/openspec/changes/manual-reminder-execution/tasks.md new file mode 100644 index 000000000..a5f224e78 --- /dev/null +++ b/openspec/changes/manual-reminder-execution/tasks.md @@ -0,0 +1,35 @@ +## 1. Planning Alignment + +- [x] 1.1 Update `docs/prd/PRD-008-scheduling-and-periodic-tasks.md` so manual CLI execution is no longer an MVP non-goal and is included in task-management acceptance criteria. +- [x] 1.2 Run `openspec validate manual-reminder-execution --type change` and resolve spec-artifact errors before coding. + +## 2. Actor Runtime + +- [x] 2.1 Add reminder execution source tracking (`scheduled` / `manual`) to the reminder actor protocol, internal completion message, and history record shape with legacy history defaulting to `scheduled`. +- [x] 2.2 Add `RunReminderNowCommand` / response handling in `ReminderManagerActor` with gates for scheduling disabled, missing reminder, disabled reminder, expired recurring reminder, same-reminder in-flight, and global concurrency full. +- [x] 2.3 Start manual runs through `ReminderExecutionActor` with no Akka.Reminders envelope, no schedule mutation, no cron reschedule, and no one-shot consumption. +- [x] 2.4 Ensure manual completion appends history but does not update scheduled consecutive-failure counters or scheduled auto-disable state. + +## 3. API and CLI + +- [x] 3.1 Add `POST /api/reminders/{id}/run` under the existing reminder route group and restrict it to Operator authority. +- [x] 3.2 Add `DaemonApi.RunReminderAsync` and `netclaw reminder run ` with clear success, daemon-offline, not-found, disabled, busy, and scheduling-disabled messages. +- [x] 3.3 Update reminder CLI help text and operator-facing output to mention immediate execution. + +## 4. Tests + +- [x] 4.1 Add actor tests proving manual run success dispatches, disabled/missing/expired/busy gates reject without dispatch/history, global concurrency rejects without queueing, and one-shot manual completion leaves the reminder enabled. +- [x] 4.2 Add actor tests proving manual success/failure leaves scheduled failure accounting unchanged and `CurrentSession` manual execution uses no scheduler ack/redelivery. +- [x] 4.3 Add history-store tests for `source` persistence and legacy records without `source` reading as `scheduled`. +- [x] 4.4 Add daemon endpoint tests for Operator authorization and command/response mapping. +- [x] 4.5 Add CLI tests for `netclaw reminder run ` success and daemon rejection output. + +## 5. Docs, Skills, and Verification + +- [x] 5.1 Update `feeds/skills/.system/files/netclaw-operations/references/scheduling.md` and bump `netclaw-operations` `metadata.version`. +- [x] 5.2 Run targeted tests for `Netclaw.Actors.Tests`, `Netclaw.Daemon.Tests`, and `Netclaw.Cli.Tests` covering the changed surfaces. +- [ ] 5.3 Run repository quality gates: `dotnet test`, `dotnet slopwatch analyze`, `pwsh ./scripts/Add-FileHeaders.ps1 -Verify`, and `git diff --check`. + - Blocked: full `dotnet test --no-restore` exhausts local disk (`No space left on device` / filesystem 100%). Completed `dotnet slopwatch analyze`, file-header verification, and `git diff --check`. +- [x] 5.4 Run `./evals/run-evals.sh` because system skill guidance changed, or document an environmental blocker with evidence. + - Blocked: eval target credentials are not configured in this environment (`NETCLAW_EVAL_PROVIDER_TYPE`, `NETCLAW_EVAL_PROVIDER_ENDPOINT`, `NETCLAW_EVAL_MODEL_ID`). +- [x] 5.5 Re-run `openspec validate manual-reminder-execution --type change` after implementation and keep artifacts aligned with code. diff --git a/src/Netclaw.Actors.Tests/Reminders/ReminderExecutionActorTests.cs b/src/Netclaw.Actors.Tests/Reminders/ReminderExecutionActorTests.cs index 8851f80da..3ecddf3fd 100644 --- a/src/Netclaw.Actors.Tests/Reminders/ReminderExecutionActorTests.cs +++ b/src/Netclaw.Actors.Tests/Reminders/ReminderExecutionActorTests.cs @@ -321,7 +321,8 @@ public ParentProxy( definition, pipeline, TimeProvider.System, - historyStore), + historyStore, + ReminderExecutionSource.Scheduled), "exec"); ReceiveAny(msg => probe.Tell(msg)); diff --git a/src/Netclaw.Actors.Tests/Reminders/ReminderHistoryStoreTests.cs b/src/Netclaw.Actors.Tests/Reminders/ReminderHistoryStoreTests.cs index 1e9874bf8..aea246835 100644 --- a/src/Netclaw.Actors.Tests/Reminders/ReminderHistoryStoreTests.cs +++ b/src/Netclaw.Actors.Tests/Reminders/ReminderHistoryStoreTests.cs @@ -91,11 +91,41 @@ public async Task Read_respects_maxRecords_limit() Assert.Equal("session-4", records[2].SessionId); } - private static HistoryRecord MakeRecord(bool success, string? sessionId = null) => + [Fact] + public async Task Append_persists_execution_source() + { + await _store.AppendAsync(TestId, MakeRecord(true, source: ReminderExecutionSource.Manual)); + + var records = await _store.ReadAsync(TestId, 10); + + Assert.Single(records); + Assert.Equal(ReminderExecutionSource.Manual, records[0].Source); + } + + [Fact] + public async Task Read_defaults_legacy_record_without_source_to_scheduled() + { + var paths = new NetclawPaths(_dir.Path); + var path = Path.Combine(paths.RemindersDirectory, $"{Uri.EscapeDataString(TestId.Value)}.history.jsonl"); + await File.WriteAllTextAsync(path, + "{\"firedAt\":\"2026-01-01T00:00:00+00:00\",\"success\":true,\"durationMs\":42,\"sessionId\":\"reminder/test-reminder/123\",\"errorMessage\":null}\n", + TestContext.Current.CancellationToken); + + var records = await _store.ReadAsync(TestId, 10); + + Assert.Single(records); + Assert.Equal(ReminderExecutionSource.Scheduled, records[0].Source); + } + + private static HistoryRecord MakeRecord( + bool success, + string? sessionId = null, + ReminderExecutionSource source = ReminderExecutionSource.Scheduled) => new( - FiredAt: DateTimeOffset.UtcNow, + FiredAt: new DateTimeOffset(2026, 1, 1, 0, 0, 0, TimeSpan.Zero), Success: success, DurationMs: 1234, SessionId: sessionId ?? "reminder/test-reminder/1234567890", - ErrorMessage: success ? null : "test error"); + ErrorMessage: success ? null : "test error", + Source: source); } diff --git a/src/Netclaw.Actors.Tests/Reminders/ReminderManagerActorTests.cs b/src/Netclaw.Actors.Tests/Reminders/ReminderManagerActorTests.cs index 2886b270b..b5ac6a48c 100644 --- a/src/Netclaw.Actors.Tests/Reminders/ReminderManagerActorTests.cs +++ b/src/Netclaw.Actors.Tests/Reminders/ReminderManagerActorTests.cs @@ -25,6 +25,7 @@ public class ReminderManagerActorTests : TestKit { private readonly string _basePath = Path.Combine(Path.GetTempPath(), $"netclaw-reminder-tests-{Guid.NewGuid():N}"); private ReminderDefinitionStore _definitionStore = null!; + private ReminderHistoryStore _historyStore = null!; private TestNotificationSink _notificationSink = null!; public ReminderManagerActorTests(ITestOutputHelper output) : base(output: output) { } @@ -42,7 +43,7 @@ protected override void ConfigureAkka(AkkaConfigurationBuilder builder, IService _definitionStore = new ReminderDefinitionStore(paths); _notificationSink = new TestNotificationSink(); var definitionStore = _definitionStore; - var historyStore = new ReminderHistoryStore(paths); + _historyStore = new ReminderHistoryStore(paths); // Wire local reminders with in-memory storage var sharedResolver = new TestShardRegionResolver(); @@ -72,7 +73,7 @@ protected override void ConfigureAkka(AkkaConfigurationBuilder builder, IService new SchedulingConfig(), TimeProvider.System, definitionStore, - historyStore, + _historyStore, _notificationSink, NullReminderChannelNotifier.Instance)), "reminder-manager-test"); @@ -1129,6 +1130,339 @@ await deliveryProbe.ExpectMsgAsync( a.Category == AlertType.ReminderExecutionFailed && a.Source == definition.Id.Value); } + [Fact] + public async Task Run_now_dispatches_current_session_manual_execution_and_keeps_oneshot_enabled() + { + var manager = await GetManagerAsync(); + var gatewayProbe = CreateTestProbe("manual-run-gateway"); + var autoAckRef = Sys.ActorOf( + Props.Create(() => new AutoAckTrustedGateway(gatewayProbe.Ref)), + "auto-ack-manual-run"); + ActorRegistry.For(Sys).Register(autoAckRef); + + var definition = CreateCurrentSessionDefinition("manual-run-oneshot", deliveryRequired: false); + _definitionStore.Save(definition); + + var response = await manager.Ask( + new RunReminderNowCommand(definition.Id), + TimeSpan.FromSeconds(5), + TestContext.Current.CancellationToken); + + Assert.True(response.Accepted); + + var delivered = await gatewayProbe.ExpectMsgAsync( + TimeSpan.FromSeconds(5), cancellationToken: TestContext.Current.CancellationToken); + Assert.Equal(definition.Delivery.SessionId, delivered.SessionId.Value); + Assert.Contains(definition.Instructions, delivered.Content); + Assert.NotNull(delivered.Source.ReminderId); + Assert.StartsWith("manual-run-oneshot:manual:", delivered.Source.ReminderId!.Value.Value); + + await AwaitAssertAsync(async () => + { + var health = await manager.Ask( + GetReminderHealthQuery.Instance, + TimeSpan.FromSeconds(3), + TestContext.Current.CancellationToken); + Assert.Equal(0, health.ActiveExecutions); + + var stored = _definitionStore.Get(definition.Id); + Assert.NotNull(stored); + Assert.True(stored!.Enabled); + + var history = await _historyStore.ReadAsync(definition.Id, 10); + Assert.Single(history); + Assert.True(history[0].Success); + Assert.Equal(ReminderExecutionSource.Manual, history[0].Source); + }, duration: TimeSpan.FromSeconds(5), cancellationToken: TestContext.Current.CancellationToken); + } + + [Fact] + public async Task Scheduled_oneshot_fire_during_manual_run_is_queued_and_runs_after_manual_completion() + { + var manager = await GetManagerAsync(); + var gatewayProbe = CreateTestProbe("manual-overlap-gateway"); + var autoAckRef = Sys.ActorOf( + Props.Create(() => new AutoAckTrustedGateway(gatewayProbe.Ref)), + "auto-ack-manual-overlap"); + ActorRegistry.For(Sys).Register(autoAckRef); + + var definition = CreateCurrentSessionDefinition("manual-overlap-oneshot", deliveryRequired: true); + _definitionStore.Save(definition); + + var manual = await manager.Ask( + new RunReminderNowCommand(definition.Id), + TimeSpan.FromSeconds(5), + TestContext.Current.CancellationToken); + Assert.True(manual.Accepted); + + var manualDelivery = await gatewayProbe.ExpectMsgAsync( + TimeSpan.FromSeconds(5), cancellationToken: TestContext.Current.CancellationToken); + Assert.StartsWith("manual-overlap-oneshot:manual:", manualDelivery.Source.ReminderId!.Value.Value); + Assert.NotNull(manualDelivery.Source.DeliveryObserver); + + manager.Tell(CreateEnvelope(definition.Id.Value)); + await gatewayProbe.ExpectNoMsgAsync(TimeSpan.FromMilliseconds(300), TestContext.Current.CancellationToken); + + manualDelivery.Source.DeliveryObserver!.Tell(new ReminderDeliveryResult( + manualDelivery.Source.ReminderId!.Value, + ChannelType.Slack, + Delivered: true, + ObservedAtMs: TimeProvider.System.GetUtcNow().ToUnixTimeMilliseconds())); + + var scheduledDelivery = await gatewayProbe.ExpectMsgAsync( + TimeSpan.FromSeconds(5), cancellationToken: TestContext.Current.CancellationToken); + Assert.StartsWith("manual-overlap-oneshot:", scheduledDelivery.Source.ReminderId!.Value.Value); + Assert.DoesNotContain(":manual:", scheduledDelivery.Source.ReminderId!.Value.Value); + Assert.NotNull(scheduledDelivery.Source.DeliveryObserver); + + scheduledDelivery.Source.DeliveryObserver!.Tell(new ReminderDeliveryResult( + scheduledDelivery.Source.ReminderId!.Value, + ChannelType.Slack, + Delivered: true, + ObservedAtMs: TimeProvider.System.GetUtcNow().ToUnixTimeMilliseconds())); + + await AwaitAssertAsync(async () => + { + var stored = _definitionStore.Get(definition.Id); + Assert.NotNull(stored); + Assert.False(stored!.Enabled); + + var history = await _historyStore.ReadAsync(definition.Id, 10); + Assert.Equal(2, history.Count); + Assert.Contains(history, record => record.Source == ReminderExecutionSource.Manual); + Assert.Contains(history, record => record.Source == ReminderExecutionSource.Scheduled); + }, duration: TimeSpan.FromSeconds(5), cancellationToken: TestContext.Current.CancellationToken); + } + + [Fact] + public async Task Run_now_rejects_missing_disabled_and_expired_reminders_without_history() + { + var manager = await GetManagerAsync(); + + var missingId = new ReminderId("manual-missing"); + var missing = await manager.Ask( + new RunReminderNowCommand(missingId), + TimeSpan.FromSeconds(5), + TestContext.Current.CancellationToken); + Assert.False(missing.Accepted); + Assert.Equal(ReminderRunNowError.NotFound, missing.Error); + + var disabled = CreateCurrentSessionDefinition("manual-disabled", deliveryRequired: false) with + { + Enabled = false + }; + _definitionStore.Save(disabled); + var disabledResponse = await manager.Ask( + new RunReminderNowCommand(disabled.Id), + TimeSpan.FromSeconds(5), + TestContext.Current.CancellationToken); + Assert.False(disabledResponse.Accepted); + Assert.Equal(ReminderRunNowError.Disabled, disabledResponse.Error); + Assert.Empty(await _historyStore.ReadAsync(disabled.Id, 10)); + + var now = TimeProvider.System.GetUtcNow(); + var expired = CreateCurrentSessionDefinition("manual-expired", deliveryRequired: false) with + { + Schedule = new ReminderSchedule + { + Type = ReminderScheduleType.Interval, + Interval = TimeSpan.FromMinutes(15), + FireAt = now.AddMinutes(15) + }, + ExpiresAt = now.AddSeconds(-1) + }; + _definitionStore.Save(expired); + var expiredResponse = await manager.Ask( + new RunReminderNowCommand(expired.Id), + TimeSpan.FromSeconds(5), + TestContext.Current.CancellationToken); + Assert.False(expiredResponse.Accepted); + Assert.Equal(ReminderRunNowError.Expired, expiredResponse.Error); + Assert.Empty(await _historyStore.ReadAsync(expired.Id, 10)); + } + + [Fact] + public async Task Run_now_rejects_duplicate_in_flight_without_incrementing_skip_count() + { + var manager = await GetManagerAsync(); + var deliveryProbe = CreateTestProbe("manual-duplicate-delivery"); + var slowGateway = Sys.ActorOf( + Props.Create(() => new NeverReplyGateway(deliveryProbe.Ref)), + "manual-duplicate-never-reply"); + ActorRegistry.For(Sys).Register(slowGateway); + + var definition = CreateCurrentSessionDefinition("manual-duplicate", deliveryRequired: false); + _definitionStore.Save(definition); + + var first = await manager.Ask( + new RunReminderNowCommand(definition.Id), + TimeSpan.FromSeconds(5), + TestContext.Current.CancellationToken); + Assert.True(first.Accepted); + + await deliveryProbe.ExpectMsgAsync( + TimeSpan.FromSeconds(5), cancellationToken: TestContext.Current.CancellationToken); + + var second = await manager.Ask( + new RunReminderNowCommand(definition.Id), + TimeSpan.FromSeconds(5), + TestContext.Current.CancellationToken); + Assert.False(second.Accepted); + Assert.Equal(ReminderRunNowError.AlreadyExecuting, second.Error); + + var status = await manager.Ask( + new GetReminderStatusQuery(definition.Id), + TimeSpan.FromSeconds(5), + TestContext.Current.CancellationToken); + Assert.Equal(0, status.SkippedDuplicates); + } + + [Fact] + public async Task Run_now_rejects_global_concurrency_full_without_queueing() + { + var manager = await GetManagerAsync(); + var deliveryProbe = CreateTestProbe("manual-busy-delivery"); + var autoAckRef = Sys.ActorOf( + Props.Create(() => new AutoAckTrustedGateway(deliveryProbe.Ref)), + "manual-busy-auto-ack"); + ActorRegistry.For(Sys).Register(autoAckRef); + + var activeDeliveries = new List(); + + for (var i = 0; i < ReminderManagerActor.MaxConcurrentExecutions; i++) + { + var definition = CreateCurrentSessionDefinition($"manual-busy-{i}", deliveryRequired: true); + _definitionStore.Save(definition); + + var accepted = await manager.Ask( + new RunReminderNowCommand(definition.Id), + TimeSpan.FromSeconds(5), + TestContext.Current.CancellationToken); + Assert.True(accepted.Accepted); + var delivered = await deliveryProbe.ExpectMsgAsync( + TimeSpan.FromSeconds(5), cancellationToken: TestContext.Current.CancellationToken); + Assert.NotNull(delivered.Source.DeliveryObserver); + activeDeliveries.Add(delivered); + } + + var busyDefinition = CreateCurrentSessionDefinition("manual-busy-rejected", deliveryRequired: false); + _definitionStore.Save(busyDefinition); + + var busy = await manager.Ask( + new RunReminderNowCommand(busyDefinition.Id), + TimeSpan.FromSeconds(5), + TestContext.Current.CancellationToken); + + Assert.False(busy.Accepted); + Assert.Equal(ReminderRunNowError.Busy, busy.Error); + Assert.Empty(await _historyStore.ReadAsync(busyDefinition.Id, 10)); + + foreach (var delivered in activeDeliveries) + { + delivered.Source.DeliveryObserver!.Tell(new ReminderDeliveryResult( + delivered.Source.ReminderId!.Value, + ChannelType.Slack, + Delivered: true, + ObservedAtMs: TimeProvider.System.GetUtcNow().ToUnixTimeMilliseconds())); + } + + await AwaitAssertAsync(async () => + { + var health = await manager.Ask( + GetReminderHealthQuery.Instance, + TimeSpan.FromSeconds(3), + TestContext.Current.CancellationToken); + Assert.Equal(0, health.ActiveExecutions); + }, duration: TimeSpan.FromSeconds(5), cancellationToken: TestContext.Current.CancellationToken); + + await deliveryProbe.ExpectNoMsgAsync(TimeSpan.FromSeconds(1), TestContext.Current.CancellationToken); + } + + [Fact] + public async Task Manual_failure_does_not_increment_scheduled_failure_count_or_disable() + { + var manager = await GetManagerAsync(); + var definition = CreateRecurringCurrentSessionDefinition("manual-failure-accounting"); + _definitionStore.Save(definition); + + manager.Tell(CreateEnvelope(definition.Id.Value)); + + await AwaitAssertAsync(async () => + { + var status = await manager.Ask( + new GetReminderStatusQuery(definition.Id), + TimeSpan.FromSeconds(3), + TestContext.Current.CancellationToken); + Assert.Equal(1, status.ConsecutiveFailures); + Assert.False(status.Executing); + }, duration: TimeSpan.FromSeconds(5), cancellationToken: TestContext.Current.CancellationToken); + + var manual = await manager.Ask( + new RunReminderNowCommand(definition.Id), + TimeSpan.FromSeconds(5), + TestContext.Current.CancellationToken); + Assert.True(manual.Accepted); + + await AwaitAssertAsync(async () => + { + var status = await manager.Ask( + new GetReminderStatusQuery(definition.Id), + TimeSpan.FromSeconds(3), + TestContext.Current.CancellationToken); + Assert.Equal(1, status.ConsecutiveFailures); + Assert.False(status.Executing); + + var stored = _definitionStore.Get(definition.Id); + Assert.NotNull(stored); + Assert.True(stored!.Enabled); + }, duration: TimeSpan.FromSeconds(5), cancellationToken: TestContext.Current.CancellationToken); + } + + [Fact] + public async Task Manual_success_does_not_reset_scheduled_failure_count() + { + var manager = await GetManagerAsync(); + var definition = CreateRecurringCurrentSessionDefinition("manual-success-accounting"); + _definitionStore.Save(definition); + + manager.Tell(CreateEnvelope(definition.Id.Value)); + + await AwaitAssertAsync(async () => + { + var status = await manager.Ask( + new GetReminderStatusQuery(definition.Id), + TimeSpan.FromSeconds(3), + TestContext.Current.CancellationToken); + Assert.Equal(1, status.ConsecutiveFailures); + Assert.False(status.Executing); + }, duration: TimeSpan.FromSeconds(5), cancellationToken: TestContext.Current.CancellationToken); + + var gatewayProbe = CreateTestProbe("manual-success-gateway"); + var autoAckRef = Sys.ActorOf( + Props.Create(() => new AutoAckTrustedGateway(gatewayProbe.Ref)), + "auto-ack-manual-success"); + ActorRegistry.For(Sys).Register(autoAckRef); + + var manual = await manager.Ask( + new RunReminderNowCommand(definition.Id), + TimeSpan.FromSeconds(5), + TestContext.Current.CancellationToken); + Assert.True(manual.Accepted); + + await gatewayProbe.ExpectMsgAsync( + TimeSpan.FromSeconds(5), cancellationToken: TestContext.Current.CancellationToken); + + await AwaitAssertAsync(async () => + { + var status = await manager.Ask( + new GetReminderStatusQuery(definition.Id), + TimeSpan.FromSeconds(3), + TestContext.Current.CancellationToken); + Assert.Equal(1, status.ConsecutiveFailures); + Assert.False(status.Executing); + }, duration: TimeSpan.FromSeconds(5), cancellationToken: TestContext.Current.CancellationToken); + } + /// /// Test-only gateway stub: handles /// by forwarding to a probe for assertions and immediately replying @@ -1221,6 +1555,20 @@ private static ReminderDefinition CreateCurrentSessionDefinition( }; } + private static ReminderDefinition CreateRecurringCurrentSessionDefinition(string id) + { + var now = TimeProvider.System.GetUtcNow(); + return CreateCurrentSessionDefinition(id, deliveryRequired: false) with + { + Schedule = new ReminderSchedule + { + Type = ReminderScheduleType.Interval, + Interval = TimeSpan.FromMinutes(30), + FireAt = now.AddMinutes(30) + } + }; + } + private static ReminderEnvelope CreateEnvelope(string reminderId) { var now = TimeProvider.System.GetUtcNow(); diff --git a/src/Netclaw.Actors/Reminders/ActiveExecutionTracker.cs b/src/Netclaw.Actors/Reminders/ActiveExecutionTracker.cs index 3ac72ca22..7f8112763 100644 --- a/src/Netclaw.Actors/Reminders/ActiveExecutionTracker.cs +++ b/src/Netclaw.Actors/Reminders/ActiveExecutionTracker.cs @@ -12,13 +12,16 @@ namespace Netclaw.Actors.Reminders; /// 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) => _executing.Add(reminderId); + public void Add(ReminderId reminderId, ReminderExecutionSource source) => _executing[reminderId] = source; + + public bool TryGetSource(ReminderId reminderId, out ReminderExecutionSource source) => + _executing.TryGetValue(reminderId, out source); /// true if the reminder was tracked and has been removed. public bool Remove(ReminderId reminderId) => _executing.Remove(reminderId); diff --git a/src/Netclaw.Actors/Reminders/ReminderExecutionActor.cs b/src/Netclaw.Actors/Reminders/ReminderExecutionActor.cs index 047fbc92f..894af6a89 100644 --- a/src/Netclaw.Actors/Reminders/ReminderExecutionActor.cs +++ b/src/Netclaw.Actors/Reminders/ReminderExecutionActor.cs @@ -55,6 +55,7 @@ internal sealed class ReminderExecutionActor : ReceiveActor private readonly ReminderDefinition _definition; private readonly ReminderHistoryStore _historyStore; private readonly TimeProvider _timeProvider; + private readonly ReminderExecutionSource _source; private readonly ReminderEnvelope? _envelope; private readonly ILoggingAdapter _log; private readonly DateTimeOffset _dispatchedAt; @@ -77,8 +78,9 @@ public static Props CreateProps( ISessionPipeline pipeline, TimeProvider timeProvider, ReminderHistoryStore historyStore, + ReminderExecutionSource source, ReminderEnvelope? envelope = null) => - Props.Create(() => new ReminderExecutionActor(executionId, definition, pipeline, timeProvider, historyStore, envelope)); + Props.Create(() => new ReminderExecutionActor(executionId, definition, pipeline, timeProvider, historyStore, source, envelope)); public ReminderExecutionActor( Guid executionId, @@ -86,12 +88,14 @@ public ReminderExecutionActor( ISessionPipeline pipeline, TimeProvider timeProvider, ReminderHistoryStore historyStore, + ReminderExecutionSource source, ReminderEnvelope? envelope = null) { _executionId = executionId; _definition = definition; _historyStore = historyStore; _timeProvider = timeProvider; + _source = source; _envelope = envelope; _dispatchedAt = timeProvider.GetUtcNow(); _log = Context.GetLogger(); @@ -120,7 +124,7 @@ public ReminderExecutionActor( 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}"); + $"ReminderExecution Dispatched: execution_id={_executionId} reminder_id={_definition.Id} title={_definition.Title} schedule_type={_definition.Schedule.Type} source={_source} dispatched_at={_dispatchedAt} delivery_kind={_definition.Delivery.Kind}"); if (RoutesBackToOriginSession) { @@ -221,15 +225,16 @@ private async Task InitializeCurrentSessionAsync() var audience = _definition.Audience; var boundary = GetPersistedBoundaryOrThrow(); - // Dedup key must be STABLE across Akka.Reminders redeliveries of the - // same fire, or the target session can't recognize a redelivery and - // 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 reminderDeliveryKey = $"{_definition.Id}:{fireTimeMs}"; + // Scheduled redelivery keys must be stable across Akka.Reminders + // retries, so use the envelope fire time when present. Manual runs + // are distinct operator actions, so use the execution ID instead of + // a millisecond timestamp that can collide under fast repeated runs. + var deliveryKeySuffix = _envelope is not null + ? _envelope.DueTimeUtc.ToUnixTimeMilliseconds().ToString() + : _source == ReminderExecutionSource.Manual + ? $"manual:{_executionId:N}" + : _dispatchedAt.ToUnixTimeMilliseconds().ToString(); + var reminderDeliveryKey = $"{_definition.Id}:{deliveryKeySuffix}"; _log.Info( "ReminderExecution CurrentSession Initialized: execution_id={ExecutionId} reminder_id={ReminderId} session_id={SessionId} origin={Origin} audience={Audience}", @@ -591,7 +596,8 @@ private void ReportAndStop(bool success, string? errorMessage = null) Success: success, DurationMs: durationMs, SessionId: _sessionIdValue ?? $"reminder/{_definition.Id}/unknown", - ErrorMessage: errorMessage); + ErrorMessage: errorMessage, + Source: _source); if (!success) { @@ -603,6 +609,7 @@ private void ReportAndStop(bool success, string? errorMessage = null) _executionId, _definition.Id, success, + _source, errorMessage)); // Drain stream stages before stopping so they complete gracefully diff --git a/src/Netclaw.Actors/Reminders/ReminderManagerActor.cs b/src/Netclaw.Actors/Reminders/ReminderManagerActor.cs index eddfe5d2c..71ed0738a 100644 --- a/src/Netclaw.Actors/Reminders/ReminderManagerActor.cs +++ b/src/Netclaw.Actors/Reminders/ReminderManagerActor.cs @@ -82,6 +82,7 @@ public ReminderManagerActor( ReceiveAsync(HandlePermanentDeleteAsync); ReceiveAsync(HandleDisableAsync); ReceiveAsync(HandleEnableAsync); + ReceiveAsync(HandleRunNowAsync); ReceiveAsync(HandleListAsync); ReceiveAsync(HandleGetAsync); @@ -541,6 +542,69 @@ private async Task HandleGetAsync(GetReminderCommand cmd) } } + private Task HandleRunNowAsync(RunReminderNowCommand cmd) + { + var replyTo = Sender; + + ReminderRunNowResponse Reject(ReminderRunNowError error, string message) => + new(cmd.Id, Accepted: false, Error: error, ErrorMessage: message); + + if (!_schedulingConfig.Enabled) + { + replyTo.Tell(Reject( + ReminderRunNowError.SchedulingDisabled, + "Scheduling is disabled; reminders cannot be run manually.")); + return Task.CompletedTask; + } + + var definition = _definitionStore.Get(cmd.Id); + if (definition is null) + { + replyTo.Tell(Reject( + ReminderRunNowError.NotFound, + $"Reminder '{cmd.Id.Value}' was not found.")); + return Task.CompletedTask; + } + + if (!definition.Enabled) + { + replyTo.Tell(Reject( + ReminderRunNowError.Disabled, + $"Reminder '{cmd.Id.Value}' is disabled.")); + return Task.CompletedTask; + } + + if (definition.Schedule.Type is not ReminderScheduleType.OneShot + && definition.ExpiresAt is { } expiresAt + && expiresAt <= _timeProvider.GetUtcNow()) + { + replyTo.Tell(Reject( + ReminderRunNowError.Expired, + $"Reminder '{cmd.Id.Value}' expired at {expiresAt:u}.")); + return Task.CompletedTask; + } + + if (_activeExecutions.IsExecuting(cmd.Id)) + { + replyTo.Tell(Reject( + ReminderRunNowError.AlreadyExecuting, + $"Reminder '{cmd.Id.Value}' is already executing.")); + return Task.CompletedTask; + } + + if (_activeExecutions.Count >= MaxConcurrentExecutions) + { + replyTo.Tell(Reject( + ReminderRunNowError.Busy, + "The reminder execution limit is full; retry when an active reminder completes.")); + return Task.CompletedTask; + } + + StartExecution(definition, ReminderExecutionSource.Manual); + replyTo.Tell(new ReminderRunNowResponse(cmd.Id, Accepted: true)); + return Task.CompletedTask; + } + private async Task HandleReminderFiredAsync(ReminderEnvelope envelope) { if (!_schedulingConfig.Enabled) @@ -586,6 +650,20 @@ private async Task HandleReminderFiredAsync(ReminderEnvelope en if (_activeExecutions.IsExecuting(reminderId)) { + if (definition.Schedule.Type == ReminderScheduleType.OneShot + && _activeExecutions.TryGetSource(reminderId, out var activeSource) + && activeSource == ReminderExecutionSource.Manual) + { + if (!_deferredQueue.Contains(reminderId)) + _deferredQueue.Enqueue(reminderId); + + _log.Info( + "One-shot reminder '{0}' fired while a manual run is active; queued scheduled occurrence behind the manual run.", + reminderId.Value); + await _client!.AckAsync(envelope); + return; + } + RecordSkippedDuplicate(reminderId, definition.Title, "scheduled"); await _client!.AckAsync(envelope); return; @@ -620,12 +698,12 @@ private async Task HandleReminderFiredAsync(ReminderEnvelope en { // CurrentSession: execution actor holds the envelope open and acks // itself once the target session has confirmed receipt. - StartExecution(definition, envelope); + StartExecution(definition, ReminderExecutionSource.Scheduled, envelope); } else { // Channel/None: ack envelope eagerly, execution tracks its own success - StartExecution(definition); + StartExecution(definition, ReminderExecutionSource.Scheduled); await _client!.AckAsync(envelope); } } @@ -678,8 +756,33 @@ private async Task HandleExecutionCompletedAsync(ReminderExecutionCompleted comp if (completed.Success) { - _failureCounts.Remove(completed.Id); - _log.Info("Reminder '{0}' execution completed successfully", completed.Id.Value); + if (completed.Source == ReminderExecutionSource.Scheduled) + _failureCounts.Remove(completed.Id); + + _log.Info("Reminder '{0}' execution completed successfully source={1}", completed.Id.Value, completed.Source); + } + else if (completed.Source == ReminderExecutionSource.Manual) + { + _log.Warning("Manual reminder '{0}' execution failed: {1}", completed.Id.Value, completed.ErrorMessage); + + _notificationSink.Emit(OperationalAlert.Create( + _timeProvider, + "reminder.execution.failed", + AlertType.ReminderExecutionFailed, + $"Manual reminder '{title}' execution failed: {completed.ErrorMessage}", + AlertSeverity.Warning, + source: completed.Id.Value, + context: new Dictionary + { + ["reminderId"] = completed.Id.Value, + ["title"] = title, + ["error"] = completed.ErrorMessage ?? "unknown", + ["executionSource"] = "manual", + })); + + PostFailureNoticeToChannel( + definition, + $"Manual reminder \"{title}\" failed: {completed.ErrorMessage ?? "unknown error"}"); } else { @@ -752,7 +855,8 @@ private async Task HandleExecutionCompletedAsync(ReminderExecutionCompleted comp // 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 }) + if (completed.Source == ReminderExecutionSource.Scheduled + && definition is { Schedule.Type: ReminderScheduleType.OneShot }) { _log.Info("One-shot reminder '{0}' completed, disabling (soft-delete)", completed.Id.Value); await DisableReminderInternalAsync(completed.Id); @@ -839,10 +943,13 @@ d.Schedule.Type is not ReminderScheduleType.OneShot && } } - private void StartExecution(ReminderDefinition definition, ReminderEnvelope? envelope = null) + private void StartExecution( + ReminderDefinition definition, + ReminderExecutionSource source, + ReminderEnvelope? envelope = null) { var executionId = Guid.NewGuid(); - _activeExecutions.Add(definition.Id); + _activeExecutions.Add(definition.Id, source); var actorName = $"exec-{SanitizeActorName(definition.Id.Value)}-{_timeProvider.GetUtcNow().ToUnixTimeMilliseconds()}"; var executionActor = Context.ActorOf( @@ -852,12 +959,13 @@ private void StartExecution(ReminderDefinition definition, ReminderEnvelope /// External message contract for . /// @@ -317,6 +335,7 @@ public sealed record CancelReminderCommand(ReminderId Id) : IReminderCommand, IN 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 RunReminderNowCommand(ReminderId Id) : IReminderCommand, INoSerializationVerificationNeeded; public sealed record ListRemindersCommand(bool IncludeDisabled = true) : IReminderQuery, INoSerializationVerificationNeeded; // ===== Queries ===== @@ -343,6 +362,12 @@ public sealed record ReminderStateResponse( DateTimeOffset? NextFire = null, string? ErrorMessage = null) : IReminderResponse, INoSerializationVerificationNeeded; +public sealed record ReminderRunNowResponse( + ReminderId Id, + bool Accepted, + ReminderRunNowError Error = ReminderRunNowError.None, + string? ErrorMessage = null) : IReminderResponse, INoSerializationVerificationNeeded; + public sealed record ReminderListResponse(IReadOnlyList Reminders) : IReminderResponse, INoSerializationVerificationNeeded; public sealed record GetReminderResponse(ReminderInfo? Reminder) : IReminderResponse, INoSerializationVerificationNeeded; @@ -453,6 +478,7 @@ internal sealed record ReminderExecutionCompleted( Guid ExecutionId, ReminderId Id, bool Success, + ReminderExecutionSource Source, string? ErrorMessage = null) : INoSerializationVerificationNeeded; // ── Execution history ── @@ -466,4 +492,5 @@ public sealed record HistoryRecord( bool Success, long DurationMs, string SessionId, - string? ErrorMessage); + string? ErrorMessage, + ReminderExecutionSource Source = ReminderExecutionSource.Scheduled); diff --git a/src/Netclaw.Cli.Tests/Reminder/ReminderCommandTests.cs b/src/Netclaw.Cli.Tests/Reminder/ReminderCommandTests.cs new file mode 100644 index 000000000..0c1c12b92 --- /dev/null +++ b/src/Netclaw.Cli.Tests/Reminder/ReminderCommandTests.cs @@ -0,0 +1,107 @@ +// ----------------------------------------------------------------------- +// +// Copyright (C) 2026 - 2026 Petabridge, LLC +// +// ----------------------------------------------------------------------- +using System.Net; +using Microsoft.Extensions.Configuration; +using Netclaw.Cli.Daemon; +using Netclaw.Cli.Reminder; +using Netclaw.Configuration; +using Netclaw.Tests.Utilities; +using Xunit; + +namespace Netclaw.Cli.Tests.Reminder; + +public sealed class ReminderCommandTests : IDisposable +{ + private readonly DisposableTempDir _dir = new(); + + public void Dispose() + { + _dir.Dispose(); + } + + [Fact] + public async Task Run_calls_daemon_run_endpoint_and_prints_success() + { + HttpRequestMessage? captured = null; + var api = CreateDaemonApi(request => + { + captured = request; + return FakeHttpMessageHandler.JsonResponse(new + { + id = "daily-summary", + started = true, + message = "Reminder 'daily-summary' run started." + }); + }); + + var result = await CaptureConsoleAsync(() => + ReminderCommand.RunAsync(["reminder", "run", "daily-summary"], api)); + + Assert.Equal(0, result.ExitCode); + Assert.Equal(HttpMethod.Post, captured?.Method); + Assert.Equal("/api/reminders/daily-summary/run", captured?.RequestUri?.AbsolutePath); + Assert.Contains("run started", result.Stdout); + Assert.Equal(string.Empty, result.Stderr); + } + + [Fact] + public async Task Run_surfaces_daemon_rejection() + { + var api = CreateDaemonApi(_ => FakeHttpMessageHandler.JsonResponse( + new { error = "Reminder 'daily-summary' is disabled." }, + HttpStatusCode.BadRequest)); + + var result = await CaptureConsoleAsync(() => + ReminderCommand.RunAsync(["reminder", "run", "daily-summary"], api)); + + Assert.Equal(1, result.ExitCode); + Assert.Equal(string.Empty, result.Stdout); + Assert.Contains("disabled", result.Stderr, StringComparison.OrdinalIgnoreCase); + Assert.DoesNotContain("run started", result.Stderr, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public async Task Run_without_daemon_api_reports_daemon_requirement() + { + var result = await CaptureConsoleAsync(() => + ReminderCommand.RunAsync(["reminder", "run", "daily-summary"], daemonApi: null)); + + Assert.Equal(1, result.ExitCode); + Assert.Equal(string.Empty, result.Stdout); + Assert.Contains("reminder run: requires a running daemon", result.Stderr); + } + + private DaemonApi CreateDaemonApi(Func handler) + { + var configuration = new ConfigurationBuilder().Build(); + var paths = new NetclawPaths(_dir.Path); + paths.EnsureDirectoriesExist(); + return new DaemonApi(new FakeHttpClientFactory(handler), configuration, paths); + } + + private static async Task CaptureConsoleAsync(Func> action) + { + var originalOut = Console.Out; + var originalErr = Console.Error; + using var stdout = new StringWriter(); + using var stderr = new StringWriter(); + + try + { + Console.SetOut(stdout); + Console.SetError(stderr); + var exitCode = await action(); + return new ConsoleResult(exitCode, stdout.ToString(), stderr.ToString()); + } + finally + { + Console.SetOut(originalOut); + Console.SetError(originalErr); + } + } + + private sealed record ConsoleResult(int ExitCode, string Stdout, string Stderr); +} diff --git a/src/Netclaw.Cli/Daemon/DaemonApi.cs b/src/Netclaw.Cli/Daemon/DaemonApi.cs index e0fa2e512..e4008f6a8 100644 --- a/src/Netclaw.Cli/Daemon/DaemonApi.cs +++ b/src/Netclaw.Cli/Daemon/DaemonApi.cs @@ -186,6 +186,13 @@ public async Task GetReminderStatusAsync(string id, Cancell return await client.GetAsync($"{_endpoint}/api/reminders/{id}/status", cts.Token); } + public async Task RunReminderAsync(string id, CancellationToken ct = default) + { + using var cts = CreateTimeoutCts(DefaultTimeout, ct); + var client = CreateHttpClient(); + return await client.PostAsync($"{_endpoint}/api/reminders/{Uri.EscapeDataString(id)}/run", content: null, cts.Token); + } + public async Task EnableReminderAsync(string id, CancellationToken ct = default) { using var cts = CreateTimeoutCts(DefaultTimeout, ct); diff --git a/src/Netclaw.Cli/Reminder/ReminderCommand.cs b/src/Netclaw.Cli/Reminder/ReminderCommand.cs index 3acc3301c..6f580c1a7 100644 --- a/src/Netclaw.Cli/Reminder/ReminderCommand.cs +++ b/src/Netclaw.Cli/Reminder/ReminderCommand.cs @@ -60,6 +60,7 @@ public static async Task RunAsync(string[] args, DaemonApi? daemonApi) "show" => await RunShowAsync(daemonApi, args), "history" => await RunHistoryAsync(daemonApi, args), "status" => await RunStatusAsync(daemonApi, args), + "run" => await RunRunAsync(daemonApi, args), _ => WriteHelp() }; } @@ -531,16 +532,18 @@ private static async Task RunHistoryAsync(DaemonApi api, string[] args) } const int colFiredAt = 25; + const int colSource = 10; const int colStatus = 8; const int colDuration = 12; - Console.WriteLine($"{"fired_at",-colFiredAt} {"status",-colStatus} {"duration_ms",-colDuration} session_id"); - Console.WriteLine(new string('-', colFiredAt + colStatus + colDuration + 34)); + Console.WriteLine($"{"fired_at",-colFiredAt} {"source",-colSource} {"status",-colStatus} {"duration_ms",-colDuration} session_id"); + Console.WriteLine(new string('-', colFiredAt + colSource + colStatus + colDuration + 36)); foreach (var r in records) { var status = r.Success ? "ok" : "failed"; - Console.WriteLine($"{r.FiredAt:u,-colFiredAt} {status,-colStatus} {r.DurationMs,-colDuration} {r.SessionId}"); + var source = r.Source.ToString().ToLowerInvariant(); + Console.WriteLine($"{r.FiredAt:u,-colFiredAt} {source,-colSource} {status,-colStatus} {r.DurationMs,-colDuration} {r.SessionId}"); } return 0; @@ -608,7 +611,8 @@ private static async Task RunStatusAsync(DaemonApi api, string[] args) { var outcome = r.Success ? "ok" : "failed"; var err = string.IsNullOrEmpty(r.ErrorMessage) ? "" : $" — {r.ErrorMessage}"; - Console.WriteLine($" {r.FiredAt:u} {outcome}{err}"); + var source = r.Source.ToString().ToLowerInvariant(); + Console.WriteLine($" {r.FiredAt:u} {source} {outcome}{err}"); } } @@ -622,6 +626,47 @@ private static async Task RunStatusAsync(DaemonApi api, string[] args) } } + private static async Task RunRunAsync(DaemonApi api, string[] args) + { + if (args.Length < 3) + { + Console.Error.WriteLine("Usage: netclaw reminder run "); + return 1; + } + + var id = args[2]; + + try + { + using var response = await api.RunReminderAsync(id); + var json = await response.Content.ReadAsStringAsync(); + var result = string.IsNullOrWhiteSpace(json) + ? default + : JsonSerializer.Deserialize(json); + + if (response.IsSuccessStatusCode) + { + if (result.ValueKind == JsonValueKind.Object && result.TryGetProperty("message", out var msg)) + Console.WriteLine(msg.GetString()); + else + Console.WriteLine($"Reminder '{id}' run started."); + return 0; + } + + if (result.ValueKind == JsonValueKind.Object && result.TryGetProperty("error", out var err)) + Console.Error.WriteLine($"[FAIL] {err.GetString()}"); + else + Console.Error.WriteLine($"[FAIL] daemon returned {(int)response.StatusCode}"); + return 1; + } + catch (Exception ex) + { + Console.Error.WriteLine($"[FAIL] unable to reach daemon: {ex.Message}"); + Console.Error.WriteLine(" fix: run `netclaw daemon start` and retry."); + return 1; + } + } + /// CLI-side projection of the daemon's reminder status JSON. private sealed record ReminderStatusView( string Id, @@ -648,6 +693,7 @@ private static int WriteHelp() 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(" run Run an enabled reminder immediately"); Console.WriteLine(); Console.WriteLine("Create options:"); Console.WriteLine(" --name Human-readable title (defaults to <id>)"); diff --git a/src/Netclaw.Daemon.Tests/Reminder/ReminderEndpointAuthorizationTests.cs b/src/Netclaw.Daemon.Tests/Reminder/ReminderEndpointAuthorizationTests.cs index e30f8d6ca..024c70ceb 100644 --- a/src/Netclaw.Daemon.Tests/Reminder/ReminderEndpointAuthorizationTests.cs +++ b/src/Netclaw.Daemon.Tests/Reminder/ReminderEndpointAuthorizationTests.cs @@ -395,6 +395,63 @@ public async Task Import_requires_authenticated_authority_context() Assert.Empty(_testActor.ReceivedMessages); } + [Fact] + public async Task NonOperator_POST_reminders_run_returns_403_and_actor_receives_no_command() + { + await using var app = await CreateAppAsync(spoofLoopback: false, addNonOperatorScheme: true); + var client = app.GetTestClient(); + client.DefaultRequestHeaders.Add(NonOperatorAuthHandler.HeaderName, NonOperatorAuthHandler.HeaderValue); + + var response = await client.PostAsync( + "/api/reminders/non-operator-run/run", + content: null, + TestContext.Current.CancellationToken); + + Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode); + Assert.Empty(_testActor.ReceivedMessages); + } + + [Fact] + public async Task Operator_POST_reminders_run_succeeds_and_actor_receives_RunReminderNowCommand() + { + await using var app = await CreateAppAsync(spoofLoopback: true); + var client = app.GetTestClient(); + + var response = await client.PostAsync( + "/api/reminders/run-ok/run", + content: null, + TestContext.Current.CancellationToken); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var body = await response.Content.ReadFromJsonAsync<JsonElement>(TestContext.Current.CancellationToken); + Assert.True(body.GetProperty("started").GetBoolean()); + Assert.Contains("run-ok", body.GetProperty("message").GetString()); + + var runCmd = _testActor.ReceivedMessages.OfType<RunReminderNowCommand>().FirstOrDefault(); + Assert.NotNull(runCmd); + Assert.Equal("run-ok", runCmd.Id.Value); + } + + [Fact] + public async Task POST_reminders_run_surfaces_actor_rejection() + { + await using var app = await CreateAppAsync(spoofLoopback: true); + var client = app.GetTestClient(); + + var response = await client.PostAsync( + "/api/reminders/disabled-run/run", + content: null, + TestContext.Current.CancellationToken); + + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + var body = await response.Content.ReadFromJsonAsync<JsonElement>(TestContext.Current.CancellationToken); + Assert.Contains("disabled", body.GetProperty("error").GetString(), StringComparison.OrdinalIgnoreCase); + + var runCmd = _testActor.ReceivedMessages.OfType<RunReminderNowCommand>().FirstOrDefault(); + Assert.NotNull(runCmd); + Assert.Equal("disabled-run", runCmd.Id.Value); + } + // ── App factory ── private async Task<WebApplication> CreateAppAsync( @@ -544,6 +601,22 @@ public RecordingReminderActor(TestReminderActor sink) sink.Record(cmd); Sender.Tell(new ReminderStateResponse(cmd.Id, Found: false, Enabled: false)); }); + + Receive<RunReminderNowCommand>(cmd => + { + sink.Record(cmd); + if (cmd.Id.Value == "disabled-run") + { + Sender.Tell(new ReminderRunNowResponse( + cmd.Id, + Accepted: false, + Error: ReminderRunNowError.Disabled, + ErrorMessage: "Reminder 'disabled-run' is disabled.")); + return; + } + + Sender.Tell(new ReminderRunNowResponse(cmd.Id, Accepted: true)); + }); } } diff --git a/src/Netclaw.Daemon/Reminders/ReminderEndpointRouteBuilderExtensions.cs b/src/Netclaw.Daemon/Reminders/ReminderEndpointRouteBuilderExtensions.cs index ce1dc6001..ef7202e82 100644 --- a/src/Netclaw.Daemon/Reminders/ReminderEndpointRouteBuilderExtensions.cs +++ b/src/Netclaw.Daemon/Reminders/ReminderEndpointRouteBuilderExtensions.cs @@ -255,6 +255,41 @@ public static IEndpointRouteBuilder MapReminderEndpoints(this IEndpointRouteBuil .WithName("EnableReminder") .WithSummary("Re-enable a previously disabled reminder."); + reminders.MapPost("/{id}/run", async ValueTask<Results<Ok<ReminderRunResponse>, NotFound<ReminderErrorResponse>, BadRequest<ReminderErrorResponse>, ProblemHttpResult>> ( + string id, + IRequiredActor<ReminderManagerActorKey> actor, + ClaimsPrincipalMapper mapper, + HttpContext httpContext, + CancellationToken ct) => + { + var authorization = ResolveReminderAuthorizationContext(mapper, httpContext); + if (authorization?.SourceAudience is null) + return TypedResults.Problem( + detail: "Running a reminder requires Operator authority.", + statusCode: StatusCodes.Status403Forbidden); + + var manager = await actor.GetAsync(ct); + var response = await manager.Ask<ReminderRunNowResponse>( + new RunReminderNowCommand(new ReminderId(id)), + TimeSpan.FromSeconds(10), + ct); + + if (response.Accepted) + { + return TypedResults.Ok(new ReminderRunResponse( + id, + Started: true, + Message: $"Reminder '{id}' run started.")); + } + + var error = response.ErrorMessage ?? $"Reminder '{id}' could not be run."; + return response.Error == ReminderRunNowError.NotFound + ? TypedResults.NotFound(new ReminderErrorResponse(error)) + : TypedResults.BadRequest(new ReminderErrorResponse(error)); + }) + .WithName("RunReminderNow") + .WithSummary("Immediately run an enabled reminder without changing its schedule (requires Operator authority)."); + reminders.MapGet("/{id}", async ValueTask<Results<Ok<ReminderDetailDto>, NotFound<ReminderErrorResponse>>> ( string id, IRequiredActor<ReminderManagerActorKey> actor, @@ -441,5 +476,8 @@ internal sealed record ReminderDisableResponse(string Id, bool Enabled, string M /// <summary>State of a reminder after an enable request.</summary> internal sealed record ReminderEnableResponse(string Id, bool Enabled, DateTimeOffset? NextFire, string Message); +/// <summary>State of a reminder after a manual run request.</summary> +internal sealed record ReminderRunResponse(string Id, bool Started, string Message); + /// <summary>Failure detail for a rejected enable request.</summary> internal sealed record ReminderEnableErrorResponse(string? Error, string Id, bool Enabled); From 4685f349f6112a1cd2eb323443106ad00e5aa013 Mon Sep 17 00:00:00 2001 From: Aaron Stannard <aaron@petabridge.com> Date: Tue, 30 Jun 2026 19:30:06 +0000 Subject: [PATCH 2/4] docs(skills): suggest reminder validation runs --- .../.system/files/netclaw-operations/SKILL.md | 6 ++++-- .../netclaw-operations/references/scheduling.md | 13 +++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/feeds/skills/.system/files/netclaw-operations/SKILL.md b/feeds/skills/.system/files/netclaw-operations/SKILL.md index b280af28c..a710b2beb 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.24.0" + version: "2.24.1" --- # Netclaw Operations @@ -52,7 +52,9 @@ it needs first with `netclaw approvals trust-verb <verb>`. Background shell: set `_background: true` on `shell_execute` (max 5 concurrent; cancel servers/watchers when done; background jobs are killed when the session passivates). Operators can run an enabled reminder immediately with `netclaw reminder run <id>`; -this requires the daemon and does not change the reminder's schedule. +this requires the daemon and does not change the reminder's schedule. After +creating a complex reminder, offer a manual validation run first; skip the prompt +for trivial check-backs. Full detail — delivery contract, proactive channel messaging, approval scoping, job lifecycle — is in diff --git a/feeds/skills/.system/files/netclaw-operations/references/scheduling.md b/feeds/skills/.system/files/netclaw-operations/references/scheduling.md index 3a012b1c8..513418552 100644 --- a/feeds/skills/.system/files/netclaw-operations/references/scheduling.md +++ b/feeds/skills/.system/files/netclaw-operations/references/scheduling.md @@ -67,6 +67,19 @@ daemon rejects manual runs when scheduling is disabled, the reminder is missing or disabled, the recurring reminder has expired, the same reminder is already executing, or the global reminder execution limit is full. +After creating a reminder, decide whether to offer an immediate validation run: + +- **Offer it for complex reminders** — shell commands, subagents, external APIs, + channel delivery, state/dedup files, or anything requiring pre-approved verbs. + Ask plainly: "This reminder has a few moving parts. Want me to run it once now + with `netclaw reminder run <id>` so we can catch approval, path, or delivery + problems before the scheduled fire?" +- **Skip it for trivial reminders** — simple conversational check-backs or + reminders with no tool use, no side effects, and no external delivery risk. +- **Do not call it a simulated dry run.** `netclaw reminder run <id>` performs a + real execution through the reminder pipeline. Warn the user if the reminder may + post to a channel, write files, call external services, or mutate state. + 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. From 8928aa9f37a077430602a09b9c44fea302f07554 Mon Sep 17 00:00:00 2001 From: Aaron Stannard <aaron@petabridge.com> Date: Tue, 7 Jul 2026 18:44:56 +0000 Subject: [PATCH 3/4] fix(reminders): harden manual run contracts --- .../manual-reminder-execution/design.md | 7 ++- .../manual-reminder-execution/proposal.md | 4 +- .../specs/netclaw-scheduling/spec.md | 18 ++++-- .../manual-reminder-execution/tasks.md | 4 +- .../Reminders/ReminderHistoryStoreTests.cs | 13 ++++ .../Reminders/ReminderManagerActorTests.cs | 48 +++++++++++---- .../Reminders/ReminderHistoryStore.cs | 2 +- .../Reminders/ReminderManagerActor.cs | 8 +++ .../Reminders/ReminderProtocol.cs | 5 +- .../Reminder/ReminderCommandTests.cs | 15 +++++ src/Netclaw.Cli/Reminder/ReminderCommand.cs | 2 + .../ReminderEndpointAuthorizationTests.cs | 50 ++++++++++++++- .../ReminderEndpointRouteBuilderExtensions.cs | 61 ++++++++++++++++--- 13 files changed, 201 insertions(+), 36 deletions(-) diff --git a/openspec/changes/manual-reminder-execution/design.md b/openspec/changes/manual-reminder-execution/design.md index 54d66fb04..873b7871e 100644 --- a/openspec/changes/manual-reminder-execution/design.md +++ b/openspec/changes/manual-reminder-execution/design.md @@ -31,9 +31,10 @@ manual diagnostic failures count as scheduled production failures. ### D1. Manual runs enter through an explicit manager command -Add `RunReminderNowCommand(ReminderId)` to the external reminder actor protocol. -The daemon endpoint sends this command to `ReminderManagerActor`; the manager -loads the persisted definition and starts execution directly. +Add `RunReminderNowCommand(ReminderId, Authorization)` to the external reminder +actor protocol. The daemon endpoint sends this command to `ReminderManagerActor`; +the manager rejects missing authorization context, loads the persisted definition, +and starts execution directly. **Rationale:** This keeps the control-plane request inside the actor that already owns duplicate execution, concurrency, expiry, and definition lookup. It avoids diff --git a/openspec/changes/manual-reminder-execution/proposal.md b/openspec/changes/manual-reminder-execution/proposal.md index 676f362cc..30e8db26b 100644 --- a/openspec/changes/manual-reminder-execution/proposal.md +++ b/openspec/changes/manual-reminder-execution/proposal.md @@ -60,8 +60,8 @@ None. ## Impact -- **Actor protocol:** add an external `RunReminderNowCommand` and response; add - execution origin to the internal completion message. +- **Actor protocol:** add an external `RunReminderNowCommand` with authorization + context and response; add execution origin to the internal completion message. - **Runtime:** `ReminderManagerActor` starts manual executions directly through `ReminderExecutionActor` with no Akka.Reminders envelope. Scheduled execution paths continue to use existing envelope ack/redelivery behavior. diff --git a/openspec/changes/manual-reminder-execution/specs/netclaw-scheduling/spec.md b/openspec/changes/manual-reminder-execution/specs/netclaw-scheduling/spec.md index e83de8a31..55d8f5b57 100644 --- a/openspec/changes/manual-reminder-execution/specs/netclaw-scheduling/spec.md +++ b/openspec/changes/manual-reminder-execution/specs/netclaw-scheduling/spec.md @@ -20,11 +20,11 @@ scheduled execution SHALL retain source `scheduled` and SHALL apply normal one-shot completion behavior. The manager SHALL reject immediate execution before dispatch when scheduling is -disabled, the reminder does not exist, the reminder is disabled, the reminder is -an expired recurring reminder, the same reminder is already executing, or the -global reminder execution concurrency limit is full. Rejected immediate -executions SHALL return a structured response with a clear reason and SHALL NOT -append execution history. +disabled, the request lacks operator authorization context, the reminder does +not exist, the reminder is disabled, the reminder is an expired recurring +reminder, the same reminder is already executing, or the global reminder +execution concurrency limit is full. Rejected immediate executions SHALL return a +structured response with a clear reason and SHALL NOT append execution history. Manual execution completion SHALL append normal execution history, but manual failure and success SHALL NOT modify the scheduled consecutive-failure counter or @@ -65,6 +65,14 @@ trigger scheduled auto-disable behavior. - **AND** no execution actor is started - **AND** no execution history is appended +#### Scenario: Manual execution rejects missing authorization context + +- **GIVEN** an enabled reminder definition exists +- **WHEN** immediate execution is requested without operator authorization context +- **THEN** the manager rejects the request before dispatch +- **AND** no execution actor is started +- **AND** no execution history is appended + #### Scenario: Manual execution rejects duplicate in-flight reminder - **GIVEN** a reminder is already executing from a scheduled or manual run diff --git a/openspec/changes/manual-reminder-execution/tasks.md b/openspec/changes/manual-reminder-execution/tasks.md index a5f224e78..309b6da94 100644 --- a/openspec/changes/manual-reminder-execution/tasks.md +++ b/openspec/changes/manual-reminder-execution/tasks.md @@ -6,7 +6,7 @@ ## 2. Actor Runtime - [x] 2.1 Add reminder execution source tracking (`scheduled` / `manual`) to the reminder actor protocol, internal completion message, and history record shape with legacy history defaulting to `scheduled`. -- [x] 2.2 Add `RunReminderNowCommand` / response handling in `ReminderManagerActor` with gates for scheduling disabled, missing reminder, disabled reminder, expired recurring reminder, same-reminder in-flight, and global concurrency full. +- [x] 2.2 Add `RunReminderNowCommand` / response handling in `ReminderManagerActor` with gates for missing authorization, scheduling disabled, missing reminder, disabled reminder, expired recurring reminder, same-reminder in-flight, and global concurrency full. - [x] 2.3 Start manual runs through `ReminderExecutionActor` with no Akka.Reminders envelope, no schedule mutation, no cron reschedule, and no one-shot consumption. - [x] 2.4 Ensure manual completion appends history but does not update scheduled consecutive-failure counters or scheduled auto-disable state. @@ -18,7 +18,7 @@ ## 4. Tests -- [x] 4.1 Add actor tests proving manual run success dispatches, disabled/missing/expired/busy gates reject without dispatch/history, global concurrency rejects without queueing, and one-shot manual completion leaves the reminder enabled. +- [x] 4.1 Add actor tests proving manual run success dispatches, missing-authorization/disabled/missing/expired/busy gates reject without dispatch/history, global concurrency rejects without queueing, and one-shot manual completion leaves the reminder enabled. - [x] 4.2 Add actor tests proving manual success/failure leaves scheduled failure accounting unchanged and `CurrentSession` manual execution uses no scheduler ack/redelivery. - [x] 4.3 Add history-store tests for `source` persistence and legacy records without `source` reading as `scheduled`. - [x] 4.4 Add daemon endpoint tests for Operator authorization and command/response mapping. diff --git a/src/Netclaw.Actors.Tests/Reminders/ReminderHistoryStoreTests.cs b/src/Netclaw.Actors.Tests/Reminders/ReminderHistoryStoreTests.cs index aea246835..915000e04 100644 --- a/src/Netclaw.Actors.Tests/Reminders/ReminderHistoryStoreTests.cs +++ b/src/Netclaw.Actors.Tests/Reminders/ReminderHistoryStoreTests.cs @@ -102,6 +102,19 @@ public async Task Append_persists_execution_source() Assert.Equal(ReminderExecutionSource.Manual, records[0].Source); } + [Fact] + public async Task Append_writes_lowercase_source_wire_value() + { + await _store.AppendAsync(TestId, MakeRecord(true, source: ReminderExecutionSource.Manual)); + + var paths = new NetclawPaths(_dir.Path); + var path = Path.Combine(paths.RemindersDirectory, $"{Uri.EscapeDataString(TestId.Value)}.history.jsonl"); + var json = await File.ReadAllTextAsync(path, TestContext.Current.CancellationToken); + + Assert.Contains("\"source\":\"manual\"", json); + Assert.DoesNotContain("\"source\":\"Manual\"", json); + } + [Fact] public async Task Read_defaults_legacy_record_without_source_to_scheduled() { diff --git a/src/Netclaw.Actors.Tests/Reminders/ReminderManagerActorTests.cs b/src/Netclaw.Actors.Tests/Reminders/ReminderManagerActorTests.cs index b5ac6a48c..0421f1333 100644 --- a/src/Netclaw.Actors.Tests/Reminders/ReminderManagerActorTests.cs +++ b/src/Netclaw.Actors.Tests/Reminders/ReminderManagerActorTests.cs @@ -30,6 +30,9 @@ public class ReminderManagerActorTests : TestKit public ReminderManagerActorTests(ITestOutputHelper output) : base(output: output) { } + private static ReminderAudienceAuthorizationContext OperatorAuthorization() => + new(TrustAudience.Team, "test-operator"); + protected override void ConfigureAkka(AkkaConfigurationBuilder builder, IServiceProvider provider) { builder @@ -1144,7 +1147,7 @@ public async Task Run_now_dispatches_current_session_manual_execution_and_keeps_ _definitionStore.Save(definition); var response = await manager.Ask<ReminderRunNowResponse>( - new RunReminderNowCommand(definition.Id), + new RunReminderNowCommand(definition.Id, OperatorAuthorization()), TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken); @@ -1176,6 +1179,29 @@ await AwaitAssertAsync(async () => }, duration: TimeSpan.FromSeconds(5), cancellationToken: TestContext.Current.CancellationToken); } + [Fact] + public async Task Run_now_rejects_missing_authorization_without_dispatch_or_history() + { + var manager = await GetManagerAsync(); + var definition = CreateDefinition("manual-missing-auth", "Check auth"); + _definitionStore.Save(definition); + + var response = await manager.Ask<ReminderRunNowResponse>( + new RunReminderNowCommand(definition.Id, Authorization: null), + TimeSpan.FromSeconds(5), + TestContext.Current.CancellationToken); + + Assert.False(response.Accepted); + Assert.Equal(ReminderRunNowError.Unauthorized, response.Error); + Assert.Empty(await _historyStore.ReadAsync(definition.Id, 10)); + + var health = await manager.Ask<ReminderHealthResponse>( + GetReminderHealthQuery.Instance, + TimeSpan.FromSeconds(3), + TestContext.Current.CancellationToken); + Assert.Equal(0, health.ActiveExecutions); + } + [Fact] public async Task Scheduled_oneshot_fire_during_manual_run_is_queued_and_runs_after_manual_completion() { @@ -1190,7 +1216,7 @@ public async Task Scheduled_oneshot_fire_during_manual_run_is_queued_and_runs_af _definitionStore.Save(definition); var manual = await manager.Ask<ReminderRunNowResponse>( - new RunReminderNowCommand(definition.Id), + new RunReminderNowCommand(definition.Id, OperatorAuthorization()), TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken); Assert.True(manual.Accepted); @@ -1241,7 +1267,7 @@ public async Task Run_now_rejects_missing_disabled_and_expired_reminders_without var missingId = new ReminderId("manual-missing"); var missing = await manager.Ask<ReminderRunNowResponse>( - new RunReminderNowCommand(missingId), + new RunReminderNowCommand(missingId, OperatorAuthorization()), TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken); Assert.False(missing.Accepted); @@ -1253,7 +1279,7 @@ public async Task Run_now_rejects_missing_disabled_and_expired_reminders_without }; _definitionStore.Save(disabled); var disabledResponse = await manager.Ask<ReminderRunNowResponse>( - new RunReminderNowCommand(disabled.Id), + new RunReminderNowCommand(disabled.Id, OperatorAuthorization()), TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken); Assert.False(disabledResponse.Accepted); @@ -1273,7 +1299,7 @@ public async Task Run_now_rejects_missing_disabled_and_expired_reminders_without }; _definitionStore.Save(expired); var expiredResponse = await manager.Ask<ReminderRunNowResponse>( - new RunReminderNowCommand(expired.Id), + new RunReminderNowCommand(expired.Id, OperatorAuthorization()), TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken); Assert.False(expiredResponse.Accepted); @@ -1295,7 +1321,7 @@ public async Task Run_now_rejects_duplicate_in_flight_without_incrementing_skip_ _definitionStore.Save(definition); var first = await manager.Ask<ReminderRunNowResponse>( - new RunReminderNowCommand(definition.Id), + new RunReminderNowCommand(definition.Id, OperatorAuthorization()), TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken); Assert.True(first.Accepted); @@ -1304,7 +1330,7 @@ await deliveryProbe.ExpectMsgAsync<DeliverTrustedSessionTurn>( TimeSpan.FromSeconds(5), cancellationToken: TestContext.Current.CancellationToken); var second = await manager.Ask<ReminderRunNowResponse>( - new RunReminderNowCommand(definition.Id), + new RunReminderNowCommand(definition.Id, OperatorAuthorization()), TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken); Assert.False(second.Accepted); @@ -1335,7 +1361,7 @@ public async Task Run_now_rejects_global_concurrency_full_without_queueing() _definitionStore.Save(definition); var accepted = await manager.Ask<ReminderRunNowResponse>( - new RunReminderNowCommand(definition.Id), + new RunReminderNowCommand(definition.Id, OperatorAuthorization()), TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken); Assert.True(accepted.Accepted); @@ -1349,7 +1375,7 @@ public async Task Run_now_rejects_global_concurrency_full_without_queueing() _definitionStore.Save(busyDefinition); var busy = await manager.Ask<ReminderRunNowResponse>( - new RunReminderNowCommand(busyDefinition.Id), + new RunReminderNowCommand(busyDefinition.Id, OperatorAuthorization()), TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken); @@ -1398,7 +1424,7 @@ await AwaitAssertAsync(async () => }, duration: TimeSpan.FromSeconds(5), cancellationToken: TestContext.Current.CancellationToken); var manual = await manager.Ask<ReminderRunNowResponse>( - new RunReminderNowCommand(definition.Id), + new RunReminderNowCommand(definition.Id, OperatorAuthorization()), TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken); Assert.True(manual.Accepted); @@ -1444,7 +1470,7 @@ await AwaitAssertAsync(async () => ActorRegistry.For(Sys).Register<SlackGatewayActorKey>(autoAckRef); var manual = await manager.Ask<ReminderRunNowResponse>( - new RunReminderNowCommand(definition.Id), + new RunReminderNowCommand(definition.Id, OperatorAuthorization()), TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken); Assert.True(manual.Accepted); diff --git a/src/Netclaw.Actors/Reminders/ReminderHistoryStore.cs b/src/Netclaw.Actors/Reminders/ReminderHistoryStore.cs index bc47551ed..b69fb3bfd 100644 --- a/src/Netclaw.Actors/Reminders/ReminderHistoryStore.cs +++ b/src/Netclaw.Actors/Reminders/ReminderHistoryStore.cs @@ -30,7 +30,7 @@ public sealed class ReminderHistoryStore private static readonly JsonSerializerOptions JsonOptions = new(JsonSerializerDefaults.Web) { - Converters = { new JsonStringEnumConverter() } + Converters = { new JsonStringEnumConverter(JsonNamingPolicy.CamelCase) } }; private readonly string _directory; diff --git a/src/Netclaw.Actors/Reminders/ReminderManagerActor.cs b/src/Netclaw.Actors/Reminders/ReminderManagerActor.cs index 71ed0738a..b844f236c 100644 --- a/src/Netclaw.Actors/Reminders/ReminderManagerActor.cs +++ b/src/Netclaw.Actors/Reminders/ReminderManagerActor.cs @@ -549,6 +549,14 @@ private Task HandleRunNowAsync(RunReminderNowCommand cmd) ReminderRunNowResponse Reject(ReminderRunNowError error, string message) => new(cmd.Id, Accepted: false, Error: error, ErrorMessage: message); + if (cmd.Authorization?.SourceAudience is null) + { + replyTo.Tell(Reject( + ReminderRunNowError.Unauthorized, + "Running a reminder requires Operator authority.")); + return Task.CompletedTask; + } + if (!_schedulingConfig.Enabled) { replyTo.Tell(Reject( diff --git a/src/Netclaw.Actors/Reminders/ReminderProtocol.cs b/src/Netclaw.Actors/Reminders/ReminderProtocol.cs index d9956070c..6903b114a 100644 --- a/src/Netclaw.Actors/Reminders/ReminderProtocol.cs +++ b/src/Netclaw.Actors/Reminders/ReminderProtocol.cs @@ -287,6 +287,7 @@ public enum ReminderExecutionSource public enum ReminderRunNowError { None, + Unauthorized, SchedulingDisabled, NotFound, Disabled, @@ -335,7 +336,9 @@ public sealed record CancelReminderCommand(ReminderId Id) : IReminderCommand, IN 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 RunReminderNowCommand(ReminderId Id) : IReminderCommand, INoSerializationVerificationNeeded; +public sealed record RunReminderNowCommand( + ReminderId Id, + ReminderAudienceAuthorizationContext? Authorization) : IReminderCommand, INoSerializationVerificationNeeded; public sealed record ListRemindersCommand(bool IncludeDisabled = true) : IReminderQuery, INoSerializationVerificationNeeded; // ===== Queries ===== diff --git a/src/Netclaw.Cli.Tests/Reminder/ReminderCommandTests.cs b/src/Netclaw.Cli.Tests/Reminder/ReminderCommandTests.cs index 0c1c12b92..5c632e3ec 100644 --- a/src/Netclaw.Cli.Tests/Reminder/ReminderCommandTests.cs +++ b/src/Netclaw.Cli.Tests/Reminder/ReminderCommandTests.cs @@ -63,6 +63,21 @@ public async Task Run_surfaces_daemon_rejection() Assert.DoesNotContain("run started", result.Stderr, StringComparison.OrdinalIgnoreCase); } + [Fact] + public async Task Run_surfaces_daemon_problem_detail() + { + var api = CreateDaemonApi(_ => FakeHttpMessageHandler.JsonResponse( + new { detail = "Reminder 'daily-summary' is already executing." }, + HttpStatusCode.Conflict)); + + var result = await CaptureConsoleAsync(() => + ReminderCommand.RunAsync(["reminder", "run", "daily-summary"], api)); + + Assert.Equal(1, result.ExitCode); + Assert.Equal(string.Empty, result.Stdout); + Assert.Contains("already executing", result.Stderr, StringComparison.OrdinalIgnoreCase); + } + [Fact] public async Task Run_without_daemon_api_reports_daemon_requirement() { diff --git a/src/Netclaw.Cli/Reminder/ReminderCommand.cs b/src/Netclaw.Cli/Reminder/ReminderCommand.cs index 6f580c1a7..1c18f191e 100644 --- a/src/Netclaw.Cli/Reminder/ReminderCommand.cs +++ b/src/Netclaw.Cli/Reminder/ReminderCommand.cs @@ -655,6 +655,8 @@ private static async Task<int> RunRunAsync(DaemonApi api, string[] args) if (result.ValueKind == JsonValueKind.Object && result.TryGetProperty("error", out var err)) Console.Error.WriteLine($"[FAIL] {err.GetString()}"); + else if (result.ValueKind == JsonValueKind.Object && result.TryGetProperty("detail", out var detail)) + Console.Error.WriteLine($"[FAIL] {detail.GetString()}"); else Console.Error.WriteLine($"[FAIL] daemon returned {(int)response.StatusCode}"); return 1; diff --git a/src/Netclaw.Daemon.Tests/Reminder/ReminderEndpointAuthorizationTests.cs b/src/Netclaw.Daemon.Tests/Reminder/ReminderEndpointAuthorizationTests.cs index 024c70ceb..1748926a9 100644 --- a/src/Netclaw.Daemon.Tests/Reminder/ReminderEndpointAuthorizationTests.cs +++ b/src/Netclaw.Daemon.Tests/Reminder/ReminderEndpointAuthorizationTests.cs @@ -430,6 +430,7 @@ public async Task Operator_POST_reminders_run_succeeds_and_actor_receives_RunRem var runCmd = _testActor.ReceivedMessages.OfType<RunReminderNowCommand>().FirstOrDefault(); Assert.NotNull(runCmd); Assert.Equal("run-ok", runCmd.Id.Value); + Assert.Equal(TrustAudience.Personal, runCmd.Authorization?.SourceAudience); } [Fact] @@ -443,13 +444,58 @@ public async Task POST_reminders_run_surfaces_actor_rejection() content: null, TestContext.Current.CancellationToken); - Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + Assert.Equal(HttpStatusCode.Conflict, response.StatusCode); var body = await response.Content.ReadFromJsonAsync<JsonElement>(TestContext.Current.CancellationToken); - Assert.Contains("disabled", body.GetProperty("error").GetString(), StringComparison.OrdinalIgnoreCase); + Assert.Contains("disabled", body.GetProperty("detail").GetString(), StringComparison.OrdinalIgnoreCase); var runCmd = _testActor.ReceivedMessages.OfType<RunReminderNowCommand>().FirstOrDefault(); Assert.NotNull(runCmd); Assert.Equal("disabled-run", runCmd.Id.Value); + Assert.Equal(TrustAudience.Personal, runCmd.Authorization?.SourceAudience); + } + + [Fact] + public async Task GET_reminders_history_returns_lowercase_source() + { + var id = new ReminderId("history-json"); + var now = _timeProvider.GetUtcNow(); + _definitionStore.Save(new ReminderDefinition + { + Id = id, + Title = "history-json", + Instructions = "check status", + Delivery = new ReminderDelivery { Kind = DeliveryKind.None }, + Schedule = new ReminderSchedule + { + Type = ReminderScheduleType.OneShot, + FireAt = now.AddMinutes(5) + }, + Audience = TrustAudience.Team, + Boundary = TrustBoundary.Team, + Enabled = true, + CreatedBy = "test", + CreatedAt = now, + UpdatedAt = now + }); + await _historyStore.AppendAsync(id, new HistoryRecord( + FiredAt: now, + Success: true, + DurationMs: 42, + SessionId: "reminder/history-json/1", + ErrorMessage: null, + Source: ReminderExecutionSource.Manual)); + + await using var app = await CreateAppAsync(spoofLoopback: true); + var client = app.GetTestClient(); + + var response = await client.GetAsync( + "/api/reminders/history-json/history", + TestContext.Current.CancellationToken); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var json = await response.Content.ReadAsStringAsync(TestContext.Current.CancellationToken); + using var doc = JsonDocument.Parse(json); + Assert.Equal("manual", doc.RootElement[0].GetProperty("source").GetString()); } // ── App factory ── diff --git a/src/Netclaw.Daemon/Reminders/ReminderEndpointRouteBuilderExtensions.cs b/src/Netclaw.Daemon/Reminders/ReminderEndpointRouteBuilderExtensions.cs index ef7202e82..70fbc2c08 100644 --- a/src/Netclaw.Daemon/Reminders/ReminderEndpointRouteBuilderExtensions.cs +++ b/src/Netclaw.Daemon/Reminders/ReminderEndpointRouteBuilderExtensions.cs @@ -270,7 +270,7 @@ public static IEndpointRouteBuilder MapReminderEndpoints(this IEndpointRouteBuil var manager = await actor.GetAsync(ct); var response = await manager.Ask<ReminderRunNowResponse>( - new RunReminderNowCommand(new ReminderId(id)), + new RunReminderNowCommand(new ReminderId(id), authorization), TimeSpan.FromSeconds(10), ct); @@ -282,10 +282,7 @@ public static IEndpointRouteBuilder MapReminderEndpoints(this IEndpointRouteBuil Message: $"Reminder '{id}' run started.")); } - var error = response.ErrorMessage ?? $"Reminder '{id}' could not be run."; - return response.Error == ReminderRunNowError.NotFound - ? TypedResults.NotFound(new ReminderErrorResponse(error)) - : TypedResults.BadRequest(new ReminderErrorResponse(error)); + return ToRunNowErrorResult(id, response); }) .WithName("RunReminderNow") .WithSummary("Immediately run an enabled reminder without changing its schedule (requires Operator authority)."); @@ -324,7 +321,7 @@ public static IEndpointRouteBuilder MapReminderEndpoints(this IEndpointRouteBuil .WithName("GetReminder") .WithSummary("Get a single reminder's full definition."); - reminders.MapGet("/{id}/history", async ValueTask<Results<Ok<IReadOnlyList<HistoryRecord>>, NotFound<ReminderErrorResponse>>> ( + reminders.MapGet("/{id}/history", async ValueTask<Results<Ok<ReminderHistoryRecordDto[]>, NotFound<ReminderErrorResponse>>> ( string id, int? last, ReminderDefinitionStore definitionStore, @@ -336,7 +333,9 @@ public static IEndpointRouteBuilder MapReminderEndpoints(this IEndpointRouteBuil return TypedResults.NotFound(new ReminderErrorResponse($"Reminder '{id}' not found.")); var maxRecords = Math.Clamp(last ?? 20, 1, 500); - var records = await historyStore.ReadAsync(rid, maxRecords); + var records = (await historyStore.ReadAsync(rid, maxRecords)) + .Select(ToHistoryRecordDto) + .ToArray(); return TypedResults.Ok(records); }) .WithName("GetReminderHistory") @@ -363,7 +362,7 @@ public static IEndpointRouteBuilder MapReminderEndpoints(this IEndpointRouteBuil NextFire: status.NextFire is null ? null : SetReminderTool.FormatTimestamp(status.NextFire), ConsecutiveFailures: status.ConsecutiveFailures, SkippedDuplicates: status.SkippedDuplicates, - RecentHistory: status.RecentHistory)); + RecentHistory: status.RecentHistory.Select(ToHistoryRecordDto).ToArray())); }) .WithName("GetReminderStatus") .WithSummary("Get per-reminder operational status: in-flight, consecutive failures, skipped fires, recent history."); @@ -371,6 +370,41 @@ public static IEndpointRouteBuilder MapReminderEndpoints(this IEndpointRouteBuil return app; } + private static Results<Ok<ReminderRunResponse>, NotFound<ReminderErrorResponse>, BadRequest<ReminderErrorResponse>, ProblemHttpResult> ToRunNowErrorResult( + string id, + ReminderRunNowResponse response) + { + var error = response.ErrorMessage ?? $"Reminder '{id}' could not be run."; + + return response.Error switch + { + ReminderRunNowError.NotFound => TypedResults.NotFound(new ReminderErrorResponse(error)), + ReminderRunNowError.Unauthorized => TypedResults.Problem(error, statusCode: StatusCodes.Status403Forbidden), + ReminderRunNowError.AlreadyExecuting => TypedResults.Problem(error, statusCode: StatusCodes.Status409Conflict), + ReminderRunNowError.Busy => TypedResults.Problem(error, statusCode: StatusCodes.Status429TooManyRequests), + ReminderRunNowError.Internal => TypedResults.Problem(error, statusCode: StatusCodes.Status500InternalServerError), + ReminderRunNowError.SchedulingDisabled or ReminderRunNowError.Disabled or ReminderRunNowError.Expired => + TypedResults.Problem(error, statusCode: StatusCodes.Status409Conflict), + _ => TypedResults.BadRequest(new ReminderErrorResponse(error)) + }; + } + + private static ReminderHistoryRecordDto ToHistoryRecordDto(HistoryRecord record) => + new( + record.FiredAt, + record.Success, + record.DurationMs, + record.SessionId, + record.ErrorMessage, + ToHistorySource(record.Source)); + + private static string ToHistorySource(ReminderExecutionSource source) => source switch + { + ReminderExecutionSource.Scheduled => "scheduled", + ReminderExecutionSource.Manual => "manual", + _ => throw new ArgumentOutOfRangeException(nameof(source), source, "Unknown reminder execution source.") + }; + private static ReminderAudienceAuthorizationContext? ResolveReminderAuthorizationContext(ClaimsPrincipalMapper mapper, HttpContext httpContext) { var identity = mapper.Map(httpContext.User); @@ -450,7 +484,16 @@ internal sealed record ReminderStatusDto( string? NextFire, int ConsecutiveFailures, int SkippedDuplicates, - IReadOnlyList<HistoryRecord> RecentHistory); + ReminderHistoryRecordDto[] RecentHistory); + +/// <summary>Wire projection of a reminder history entry.</summary> +internal sealed record ReminderHistoryRecordDto( + DateTimeOffset FiredAt, + bool Success, + long DurationMs, + string SessionId, + string? ErrorMessage, + string Source); /// <summary>Acknowledgement carrying a human-readable message.</summary> internal sealed record ReminderMessageResponse(string Message); From 9448870e2b2aafa4ae71af1cd141f00af1c58256 Mon Sep 17 00:00:00 2001 From: Aaron Stannard <aaron@petabridge.com> Date: Tue, 7 Jul 2026 19:41:21 +0000 Subject: [PATCH 4/4] docs(reminders): clarify fire-now approval limits --- BACKLOG_PARKING_LOT.md | 3 +++ .../.system/files/netclaw-operations/SKILL.md | 13 ++++++----- .../references/scheduling.md | 22 +++++++++++-------- .../manual-reminder-execution/design.md | 3 +++ 4 files changed, 26 insertions(+), 15 deletions(-) diff --git a/BACKLOG_PARKING_LOT.md b/BACKLOG_PARKING_LOT.md index 3bb5f6850..09d2c9184 100644 --- a/BACKLOG_PARKING_LOT.md +++ b/BACKLOG_PARKING_LOT.md @@ -14,6 +14,9 @@ explicitly changes priority. - Fixed-length approval button labels and richer approval UI. - Config hot-reload beyond startup-time configuration. - Operator diagnostics refinements beyond current CLI/doctor/status work. +- Unattended approval escalation for reminders/webhooks: pause the autonomous + run, route approval prompts to a live operator channel, and resume under the + original autonomous execution context. ## LATER Candidates diff --git a/feeds/skills/.system/files/netclaw-operations/SKILL.md b/feeds/skills/.system/files/netclaw-operations/SKILL.md index a710b2beb..2e855c128 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.24.1" + version: "2.24.2" --- # Netclaw Operations @@ -47,14 +47,15 @@ allowed roots); the project's identity file (`.netclaw/AGENTS.md`, `CLAUDE.md`, Reminders: `set_reminder` with schedule type `once` / `interval` / `cron`. Always set `delivery_kind` explicitly (`current_session` / `channel` / `none`). A reminder -that fires unattended cannot answer approval prompts, so pre-approve any shell verbs -it needs first with `netclaw approvals trust-verb <verb>`. Background shell: set +that fires unattended cannot currently answer approval prompts, so pre-approve any +obvious shell verbs it needs first with `netclaw approvals trust-verb <verb>`. +Background shell: set `_background: true` on `shell_execute` (max 5 concurrent; cancel servers/watchers when done; background jobs are killed when the session passivates). Operators can run an enabled reminder immediately with `netclaw reminder run <id>`; -this requires the daemon and does not change the reminder's schedule. After -creating a complex reminder, offer a manual validation run first; skip the prompt -for trivial check-backs. +this requires the daemon and does not change the reminder's schedule. After creating +a complex out-of-session reminder, offer a manual fire-now run; never offer this for +`current_session` reminders, and do not describe it as approval collection. Full detail — delivery contract, proactive channel messaging, approval scoping, job lifecycle — is in diff --git a/feeds/skills/.system/files/netclaw-operations/references/scheduling.md b/feeds/skills/.system/files/netclaw-operations/references/scheduling.md index 513418552..c74b185f1 100644 --- a/feeds/skills/.system/files/netclaw-operations/references/scheduling.md +++ b/feeds/skills/.system/files/netclaw-operations/references/scheduling.md @@ -67,18 +67,22 @@ daemon rejects manual runs when scheduling is disabled, the reminder is missing or disabled, the recurring reminder has expired, the same reminder is already executing, or the global reminder execution limit is full. -After creating a reminder, decide whether to offer an immediate validation run: - -- **Offer it for complex reminders** — shell commands, subagents, external APIs, - channel delivery, state/dedup files, or anything requiring pre-approved verbs. - Ask plainly: "This reminder has a few moving parts. Want me to run it once now - with `netclaw reminder run <id>` so we can catch approval, path, or delivery - problems before the scheduled fire?" -- **Skip it for trivial reminders** — simple conversational check-backs or - reminders with no tool use, no side effects, and no external delivery risk. +After creating a reminder, decide whether to offer an immediate fire-now run: + +- **Never offer it for `delivery_kind=current_session`.** Those reminders are + conversational check-backs in the active session, not unattended execution. +- **Offer it for complex out-of-session reminders** — `delivery_kind=channel` or + `none` with shell commands, subagents, external APIs, channel delivery, + state/dedup files, or pre-approved verbs. Ask plainly: "This reminder has a few + moving parts. Want me to run it once now with `netclaw reminder run <id>` so we + can exercise the autonomous path before the scheduled fire?" - **Do not call it a simulated dry run.** `netclaw reminder run <id>` performs a real execution through the reminder pipeline. Warn the user if the reminder may post to a channel, write files, call external services, or mutate state. +- **Do not promise approval collection.** Current unattended reminder executions + cannot route approval prompts back to a live operator. A manual fire-now run can + expose missing approvals by failing visibly, but it does not solve unattended + approval escalation. Reminders that hit 5 consecutive execution failures are auto-disabled with a `ReminderAutoDisabled` critical alert. The definition stays on disk so the diff --git a/openspec/changes/manual-reminder-execution/design.md b/openspec/changes/manual-reminder-execution/design.md index 873b7871e..4b633ceab 100644 --- a/openspec/changes/manual-reminder-execution/design.md +++ b/openspec/changes/manual-reminder-execution/design.md @@ -26,6 +26,9 @@ manual diagnostic failures count as scheduled production failures. - No idempotency key or retry de-duplication for repeated CLI/API calls. - No arbitrary prompt override or one-off unsaved reminder execution. - No dashboard, Slack slash command, or TUI surface. +- No unattended approval escalation. If a manual or scheduled autonomous run hits + an approval gate today, this change may surface that failure sooner, but it does + not route the prompt back to a live operator channel. ## Decisions