feat(reminders): implement structured delivery contract (closes #690, folds in #644)#692
Merged
Aaronontheweb merged 11 commits intodevfrom Apr 21, 2026
Merged
Conversation
Addresses #690 and folds in #644. Creates the `reminder-delivery-contract` OpenSpec change with full artifact set (proposal, design, specs delta, tasks). Replaces the coupled `reportToChannel` / `notifyInstructions` / `notifyPolicy` surface on `set_reminder` with four structured fields: - `delivery: { kind, transport?, address? }` — kind is a required enum (`current_session` / `channel` / `none`) that directly selects execution mode (no more inference from populated fields) - `deliveryRequired: bool` — policy - `resultTemplate: string?` — content guidance only, never routing Adds transport-keyed `IReminderTargetResolver` (`Transport` property) as the structural hook for #644 so the next transport ships without another schema rewrite. Closes the silent-failure gap observed in journal session `D0AC6CKBK5K/1776697725.361339` by introducing a `ReminderDeliveryObserved` outbound gateway signal: for `CurrentSession` with `DeliveryRequired = true`, the execution actor waits for the session's assistant reply to actually surface through the channel before acking the envelope. Miss → execution failed, alert fires, Akka.Reminders redelivers. Netclaw is pre-production with N=1. Stale reminder rows are hard-deleted at startup; no migration code. Also archives `reminder-audience-authorization` (previously complete but unarchived) and `reminder-session-reentry` (60/67 tasks; remaining tasks were verify/sync/archive bookkeeping + three follow-up issues, two of which were YAGNI and one — SignalR channel extraction — is filed as #691). Both use `--skip-specs` because the main specs were already synced. No code changes in this commit — planning artifacts only.
- Rename ResultTemplate → ResultGuidance for clearer intent - Clarify that DeliveryRequired is ignored for None deliveries - Move duplicate transport detection to startup (fail-fast on boot) - Add session rehydration note to skill guidance
Clearer semantics: field provides guidance for what to include in the delivery to the user, not guidance about task output in general.
Replace the inference-based reminder routing model with an explicit DeliveryKind-based contract. The mixed ReportToChannel/NotifyInstructions/ NotifyPolicy surface is replaced with: - DeliveryKind enum: CurrentSession, Channel, None - ReminderDelivery struct: Kind, Transport, Address, SessionId, OriginChannelType - DeliveryRequired: bool (replaces NotificationPolicy enum) - DeliveryInstructions: content guidance (was NotifyInstructions) Key changes: - SetReminderTool now requires explicit delivery_kind parameter - IReminderTargetResolver gains Transport property for keyed dispatch - ReminderManagerActor branches on Delivery.Kind instead of field inference - ReminderExecutionActor renamed Mode B → CurrentSession delivery path - ExecutionOutputAccumulator updated for explicit delivery semantics - All tests updated for new API surface This is a breaking schema change; existing reminders will fail deserialization and be dropped at startup (N=1 pre-production user, hard-delete acceptable). Closes #690, folds in #644
Replace silent default case with ArgumentOutOfRangeException to catch bugs early if new DeliveryKind values are added without updating this switch expression.
…l names - Use ChannelType.ToWireValue() constants instead of raw "signalr"/"tui" strings - Add ReminderDelivery.GetNotificationToolName() to derive tool from transport - ReminderExecutionActor now uses transport-derived notification tool name instead of hardcoded "send_slack_message" This completes task 5.7 (transport-derived accumulator tool name).
Aaronontheweb
changed the title
The /api/reminders POST endpoint was still passing old params (ReportToChannel, NotifyInstructions, NotifyPolicy) to SetReminderTool which now requires the new delivery contract fields (DeliveryKind, DeliveryTransport, DeliveryAddress, DeliveryRequired, DeliveryInstructions). Changes: - Add new delivery fields to CreateReminderRequest - Keep legacy fields for backward compatibility - Endpoint maps legacy fields to new contract when new fields not provided - Default DeliveryKind to "none" when no delivery target specified This fixes the smoke test failure where CLI reminder create was missing the required DeliveryKind parameter.
Drop backward compatibility - no external consumers exist yet. CLI now uses: --delivery <kind> (none, channel) --transport <slack> --address <target> REST endpoint uses DeliveryKind (required), DeliveryTransport, DeliveryAddress directly. Legacy reportToChannel/NotifyInstructions fields removed.
Aaronontheweb
added a commit
that referenced
this pull request
Apr 29, 2026
) * chore(openspec): archive 19 completed changes and sync delta specs Archive all implemented OpenSpec changes, syncing their delta specs to main specs in chronological order before moving to archive. Resolves 13 cross-change spec conflicts by applying older changes first so newer implementations take precedence. Archived changes: - add-discord-init-reminder-dm-support - background-job-execution - channel-ingress-attachments - channels-content-delivery-guarantees - containerize-daemon-and-evals - device-pairing - discord-channel-with-interactions - exposure-modes - graceful-config-restart-drain - hub-auth-framework - inbound-webhooks - mcp-audience-tool-grants - mcp-server-approval-defaults - multi-speaker-attribution - public-audience-security-hardening - session-cwd-tracking - structured-tool-call-metadata - tool-approval-gates - working-context-grounding New main specs created: audience-context-filtering, daemon-container, daemon-exposure, device-pairing, feature-selection-wizard, hub-auth, inbound-webhooks, netclaw-discord-socket, project-instructions, session-cwd, tool-call-metadata Remaining active changes: reminder-delivery-contract (partial), subagent-explicit-model-selection (blocked on #648) * chore(openspec): archive reminder-delivery-contract and sync specs Implementation is complete (PR #692, #752). Remaining eval cases tracked in #793.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replaces the coupled
reportToChannel/notifyInstructions/notifyPolicysurface onset_reminderwith a structured delivery contract. Addresses #690 and folds in #644 (multi-transport routing).What Changed
New delivery contract
delivery: { kind, transport?, address? }—kindis a required enum (current_session/channel/none) that directly selects execution mode. No more inference from which optional field happens to be populated.deliveryRequired: bool— policy, defaulttrue.deliveryInstructions: string?— content guidance only, never routing.Transport-keyed resolver dispatch
Adds
IReminderTargetResolver.Transportproperty so the tool dispatches by transport key. UsesChannelType.ToWireValue()for transport comparison (no stringly-typed checks).ReminderDelivery.GetNotificationToolName()centralizes tool name derivation.Protocol changes
DeliveryKindenum andReminderDeliveryclass toReminderProtocol.csReminderDefinitionnow usesDeliverystruct instead of separateSessionId/OriginChannelType/ReportToChannelfieldsReminderInfoupdated to exposeDelivery,DeliveryRequired,DeliveryInstructionsExecution changes
ReminderManagerActor: Dispatches based onDelivery.Kindinstead of inferring modeReminderExecutionActor: UsesRoutesBackToOriginSessionproperty; throws on unexpectedDeliveryKindExecutionOutputAccumulator: Accepts notification tool name as constructor parameterOpenSpec artifacts
Planning artifacts in
openspec/changes/reminder-delivery-contract/with proposal, design, specs, and tasks.Archived predecessors:
reminder-audience-authorizationreminder-session-reentryRemaining Work (tracked in tasks.md)
ReminderDeliveryObservedsignal for delivery confirmationRelated
Test plan
dotnet build— 0 warnings, 0 errorsdotnet slopwatch analyze— verify no new violations