From abb311f5784af15b7d3cd630b8d52ef8d674a46f Mon Sep 17 00:00:00 2001
From: Gabriel Birman <25272206+gbirman@users.noreply.github.com>
Date: Mon, 21 Sep 2026 19:13:52 +0000
Subject: [PATCH 1/5] feat(reminders): make scheduling drafts reliable
---
.../reminders/ReminderComposerModal.test.tsx | 167 +++--
.../reminders/ReminderComposerModal.tsx | 122 ++--
.../reminders/ReminderEditorSplit.tsx | 46 +-
.../features/reminders/ReminderForm.test.tsx | 296 ++++++++
.../src/features/reminders/ReminderForm.tsx | 651 +++++++++++++-----
.../reminders/reminder-composer.test.ts | 4 +-
.../features/reminders/reminder-composer.ts | 7 +-
.../reminders/reminder-schedule.test.ts | 101 +++
.../features/reminders/reminder-schedule.ts | 151 ++++
apps/web/src/lib/core/util/cron.test.ts | 15 +
apps/web/src/lib/core/util/cron.ts | 5 +
.../core/util/dateSearch/dateParser.test.ts | 10 +
.../lib/core/util/dateSearch/dateParser.ts | 8 +-
docs/AGENT_GUIDE/README.md | 1 +
docs/AGENT_GUIDE/reminders.md | 51 ++
15 files changed, 1366 insertions(+), 269 deletions(-)
create mode 100644 apps/web/src/features/reminders/ReminderForm.test.tsx
create mode 100644 docs/AGENT_GUIDE/reminders.md
diff --git a/apps/web/src/features/reminders/ReminderComposerModal.test.tsx b/apps/web/src/features/reminders/ReminderComposerModal.test.tsx
index 974905c471b..2b44620c80c 100644
--- a/apps/web/src/features/reminders/ReminderComposerModal.test.tsx
+++ b/apps/web/src/features/reminders/ReminderComposerModal.test.tsx
@@ -5,7 +5,7 @@ import {
screen,
waitFor,
} from '@solidjs/testing-library';
-import type { ParentProps } from 'solid-js';
+import { createSignal, type ParentProps, Show } from 'solid-js';
import { afterEach, beforeEach, expect, it, vi } from 'vitest';
import { ReminderComposerModal } from './ReminderComposerModal';
import {
@@ -16,21 +16,19 @@ import {
const mocks = vi.hoisted(() => ({
save: vi.fn(),
- pending: false,
- failure: vi.fn(),
+ success: vi.fn(),
}));
+
vi.mock('@queries/reminders/reminders', () => ({
reminderTarget: vi.fn(),
- useCreateReminderMutation: () => ({
- mutateAsync: mocks.save,
- get isPending() {
- return mocks.pending;
- },
- }),
+ useCreateReminderMutation: () => ({ mutateAsync: mocks.save }),
}));
vi.mock('@queries/soup/cache', () => ({ refetchSoupEntity: vi.fn() }));
+vi.mock('../../lib/signals/splitLayout', () => ({
+ globalSplitManager: () => undefined,
+}));
vi.mock('@core/component/Toast/Toast', () => ({
- toast: { success: vi.fn(), failure: mocks.failure },
+ toast: { success: mocks.success, failure: vi.fn() },
}));
vi.mock('@entity/components/EntitySelectionBadge', () => ({
EntitySelectionBadge: () => null,
@@ -47,57 +45,148 @@ vi.mock('@ui', () => {
};
});
vi.mock('./ReminderForm', () => ({
- ReminderForm: (props: { onSubmit: (values: unknown) => void }) => (
-
-
-
- ),
+ setDescription(event.currentTarget.value)}
+ />
+
+
+ {props.error}
+
+
+ );
+ },
}));
+
+function savedReminder() {
+ return {
+ id: 'reminder-1',
+ schedule: { type: 'once', remindAt: '2099-01-01T09:00:00Z' },
+ };
+}
+
beforeEach(() => {
mocks.save.mockReset();
- mocks.pending = false;
- mocks.failure.mockClear();
+ mocks.success.mockReset();
closeReminderComposer();
});
+
afterEach(() => {
cleanup();
closeReminderComposer();
});
-it('dismisses before saving and calls the captured handler after success', async () => {
- let resolve!: (value: object) => void;
+
+it('keeps the draft open and prevents concurrent duplicate submits', async () => {
+ let rejectRequest!: (reason: Error) => void;
mocks.save.mockReturnValueOnce(
- new Promise((done) => {
- resolve = done;
+ new Promise((_resolve, reject) => {
+ rejectRequest = reject;
})
);
const onCreated = vi.fn();
openStandaloneReminderComposer({ onCreated });
render(() => );
- fireEvent.click(screen.getByRole('button', { name: 'Submit' }));
- expect(reminderComposerOpen()).toBe(false);
+
+ const input = screen.getByRole('textbox', {
+ name: 'Reminder description',
+ });
+ fireEvent.input(input, { target: { value: 'Keep this draft' } });
+ input.focus();
+ const form = screen.getByRole('form', { name: 'Reminder form' });
+ fireEvent.submit(form);
+ fireEvent.submit(form);
+
+ expect(mocks.save).toHaveBeenCalledOnce();
+ expect(reminderComposerOpen()).toBe(true);
+ expect(
+ (screen.getByRole('button', { name: 'Submit' }) as HTMLButtonElement)
+ .disabled
+ ).toBe(true);
+ expect((input as HTMLInputElement).disabled).toBe(true);
+
+ rejectRequest(new Error('offline'));
+ const error = await screen.findByRole('alert');
+ expect(error.textContent).toContain('Your draft is still here');
+ expect(error.textContent).toContain('may already exist');
+ expect((input as HTMLInputElement).value).toBe('Keep this draft');
+ expect((input as HTMLInputElement).disabled).toBe(false);
+ expect(document.activeElement).toBe(input);
+ expect(reminderComposerOpen()).toBe(true);
expect(onCreated).not.toHaveBeenCalled();
- resolve({});
- await waitFor(() => expect(onCreated).toHaveBeenCalledOnce());
});
-it('reports a failed save by toast without reopening or calling the handler', async () => {
- mocks.save.mockRejectedValueOnce(new Error('offline'));
+
+it('retries a rejected mutation with the same draft and closes only on success', async () => {
+ mocks.save
+ .mockRejectedValueOnce(new Error('offline'))
+ .mockResolvedValueOnce(savedReminder());
const onCreated = vi.fn();
openStandaloneReminderComposer({ onCreated });
render(() => );
- fireEvent.click(screen.getByRole('button', { name: 'Submit' }));
- await waitFor(() =>
- expect(mocks.failure).toHaveBeenCalledWith('Failed to create reminder')
+
+ const input = screen.getByRole('textbox', {
+ name: 'Reminder description',
+ });
+ fireEvent.input(input, { target: { value: 'Retry this reminder' } });
+ const form = screen.getByRole('form', { name: 'Reminder form' });
+ fireEvent.submit(form);
+ await screen.findByRole('alert');
+
+ fireEvent.submit(form);
+ await waitFor(() => expect(reminderComposerOpen()).toBe(false));
+
+ expect(mocks.save).toHaveBeenCalledTimes(2);
+ expect(mocks.save.mock.calls[1]?.[0]).toMatchObject({
+ description: 'Retry this reminder',
+ });
+ expect(onCreated).toHaveBeenCalledOnce();
+ expect(mocks.success).toHaveBeenCalledWith(
+ expect.stringContaining('Reminder set ·'),
+ expect.objectContaining({
+ actions: [expect.objectContaining({ label: 'View' })],
+ })
);
- expect(reminderComposerOpen()).toBe(false);
+});
+
+it('waits for a deferred mutation before closing and running the follow-up', async () => {
+ let resolveRequest!: (value: ReturnType) => void;
+ mocks.save.mockReturnValueOnce(
+ new Promise((resolve) => {
+ resolveRequest = resolve;
+ })
+ );
+ const onCreated = vi.fn();
+ openStandaloneReminderComposer({ onCreated });
+ render(() => );
+
+ fireEvent.submit(screen.getByRole('form', { name: 'Reminder form' }));
+ expect(reminderComposerOpen()).toBe(true);
expect(onCreated).not.toHaveBeenCalled();
+
+ resolveRequest(savedReminder());
+ await waitFor(() => expect(reminderComposerOpen()).toBe(false));
+ expect(onCreated).toHaveBeenCalledOnce();
});
diff --git a/apps/web/src/features/reminders/ReminderComposerModal.tsx b/apps/web/src/features/reminders/ReminderComposerModal.tsx
index c6ff6d6598b..88efa6bef27 100644
--- a/apps/web/src/features/reminders/ReminderComposerModal.tsx
+++ b/apps/web/src/features/reminders/ReminderComposerModal.tsx
@@ -6,9 +6,12 @@ import {
useCreateReminderMutation,
} from '@queries/reminders/reminders';
import { refetchSoupEntity } from '@queries/soup/cache';
+import type { CreateReminderRequest } from '@service-storage/generated/schemas/createReminderRequest';
+import type { Reminder } from '@service-storage/generated/schemas/reminder';
import type { ReminderSchedule } from '@service-storage/generated/schemas/reminderSchedule';
import { ActionDialogShell, Dialog } from '@ui';
-import { Show } from 'solid-js';
+import { createSignal, Show } from 'solid-js';
+import { globalSplitManager } from '../../lib/signals/splitLayout';
import { ReminderForm } from './ReminderForm';
import {
closeReminderComposer,
@@ -17,10 +20,14 @@ import {
takeReminderCreatedHandler,
} from './reminder-composer';
import {
+ describeReminderConfirmation,
resolveReminderDescription,
resolveStandaloneDescription,
} from './reminder-schedule';
+const CREATE_FAILURE_MESSAGE =
+ 'We couldn’t save this reminder. Your draft is still here—try again. If the request timed out, it may already exist; check Reminders before retrying.';
+
/**
* Creates a reminder — one about an entity, or one about nothing at all — in a
* single panel. Editing an existing reminder happens in its own split view
@@ -38,6 +45,61 @@ export function ReminderComposerModal() {
const entity = () => reminderComposerState.entity;
const standalone = () => reminderComposerState.standalone === true;
+ const [submitting, setSubmitting] = createSignal(false);
+ const [saveError, setSaveError] = createSignal();
+ let focusBeforeSave: HTMLElement | undefined;
+
+ const save = async (args: CreateReminderRequest) => {
+ if (submitting()) return;
+ focusBeforeSave =
+ document.activeElement instanceof HTMLElement
+ ? document.activeElement
+ : undefined;
+ setSubmitting(true);
+ setSaveError(undefined);
+
+ let reminder: Reminder;
+ try {
+ reminder = await createReminder.mutateAsync(args);
+ } catch {
+ setSaveError(CREATE_FAILURE_MESSAGE);
+ setSubmitting(false);
+ queueMicrotask(() => {
+ if (focusBeforeSave?.isConnected) focusBeforeSave.focus();
+ });
+ return;
+ }
+
+ const onCreated = takeReminderCreatedHandler();
+ setSubmitting(false);
+ closeReminderComposer();
+ toast.success(
+ `Reminder set · ${describeReminderConfirmation(reminder.schedule)}`,
+ {
+ actions: [
+ {
+ label: 'View',
+ onClick: () =>
+ globalSplitManager()?.openWithSplit(
+ {
+ type: 'component',
+ id: `reminder-view~${reminder.id}`,
+ },
+ { activate: true }
+ ),
+ },
+ ],
+ }
+ );
+ // This host-owned follow-up runs only after persistence. It is intentionally
+ // outside the request catch: a downstream row action failing does not mean
+ // the reminder failed to save and must never invite a duplicate retry.
+ try {
+ await onCreated?.();
+ } catch {
+ toast.failure('Reminder saved, but the source could not be updated');
+ }
+ };
const submitCreate = async (
schedule: ReminderSchedule,
@@ -46,27 +108,12 @@ export function ReminderComposerModal() {
) => {
const resolved = resolveReminderDescription(input, target);
const attachTo = reminderTarget(target);
- // Taken before the close, which clears it.
- const onCreated = takeReminderCreatedHandler();
- closeReminderComposer();
-
- try {
- await createReminder.mutateAsync({
- description: resolved,
- schedule,
- // Both or neither: the API rejects one without the other.
- ...(attachTo ?? undefined),
- });
- toast.success('Reminder set');
- } catch {
- toast.failure('Failed to create reminder');
- return;
- }
-
- // Whatever the invoking surface does with its row now that the reminder
- // will bring it back — marking it done, in every soup list. Runs only once
- // the reminder exists, so a failed create leaves the row alone.
- await onCreated?.();
+ await save({
+ description: resolved,
+ schedule,
+ // Both or neither: the API rejects one without the other.
+ ...(attachTo ?? undefined),
+ });
};
/**
@@ -85,20 +132,7 @@ export function ReminderComposerModal() {
// on it rather than a `!` on the value above.
if (!resolved) return;
- // Taken before the close, which clears it. Nothing passes one today, but
- // taking it is what keeps a handler from leaking into the next open.
- const onCreated = takeReminderCreatedHandler();
- closeReminderComposer();
-
- try {
- await createReminder.mutateAsync({ description: resolved, schedule });
- toast.success('Reminder set');
- } catch {
- toast.failure('Failed to create reminder');
- return;
- }
-
- await onCreated?.();
+ await save({ description: resolved, schedule });
};
const handleSubmit = (values: {
@@ -123,10 +157,13 @@ export function ReminderComposerModal() {