diff --git a/KNOWN-ISSUES.md b/KNOWN-ISSUES.md index e41f5ae..3500667 100644 --- a/KNOWN-ISSUES.md +++ b/KNOWN-ISSUES.md @@ -10,6 +10,8 @@ pending**. Every entry below was exercised against 0.16.19 on the date it names, and the dates still say so: the upgrade was read against the 0.16.19→0.16.20 diff rather than re-run, and nothing in it touches the session capabilities, blob, quota, submission or registry paths these entries describe. +The calendar entries below carrying a 2026-08-31 date are the exception: those +were exercised against the live 0.16.20 directly. What remains here is not a list of unknowns but of things worth knowing — where Stalwart departs from a spec, where a setting has to be turned on for a feature to work, and what ihasmail deliberately does not do. @@ -40,6 +42,10 @@ works the same way — and dropped where 0.15 was the whole subject. Support for - **Self-service credentials** — the registry path is **confirmed live** against Stalwart 0.16.19 (2026-08-25): app passwords created and revoked, password changed, 2FA enabled and disabled, with the browser session surviving the switch to an app password. The 0.15 REST path was confirmed live too, on 0.15.5 (2026-08-24), and has since been removed along with the rest of 0.15 support. The mock enforces the same rules the real server does (current password required, password policy, a TOTP code on every request once 2FA is on, app passwords exempt from it). Password changes are refused by Stalwart for accounts backed by an external directory (LDAP/SQL/OIDC); the server's own message is shown when that happens. - **Scheduled send needs one setting turned on, and says nothing when it is off.** Stalwart advertises the delay in the account's `urn:ietf:params:jmap:submission` capability — `maxDelayedSend: 2592000` (30 days) and `FUTURERELEASE` among its `submissionExtensions`, and note it is the *account* capability, not the session-level one, which is empty. But the MTA only honours a hold when `futureRelease` is set under the session's MTA extensions, and [that setting defaults to `false`](https://stalw.art/docs/ref/object/mta-extensions/). With it off, Stalwart takes the `HOLDUNTIL` parameter, skips the hold and sends the message immediately **without an error** — the capability still says thirty days. So set `futureRelease` (to the longest hold you want to allow) before relying on this; a value shorter than 30 days is fine, and a request past it is refused honestly, with a `forbiddenMailFrom` naming the limit. `npm run dev:mock:no-future-release` reproduces the silent-drop case. ihasmail asks for the delay the way JMAP requires — a `HOLDUNTIL` parameter on the envelope's `mailFrom`, since RFC 8621 makes `sendAt` read-only and server-derived — and files the held message in a **Scheduled** folder, because `onSuccessUpdateEmail` would otherwise drop it in Sent the moment the submission is created. Nothing moves it out when the hold expires, so ihasmail reconciles the folder on the way in: released messages to Sent, cancelled ones back to Drafts. Three fixes this depends on landed in **0.16.17**, below the live instance's 0.16.19: `HOLDUNTIL` taking RFC 3339 date-times again (0.16.16 had it wanting Unix timestamps), `EmailSubmission/query` on `undoStatus` agreeing with `/get` about held submissions, and `EmailSubmission/get` without `ids` iterating the right index. The hold itself is now **confirmed against the live 0.16.19** (2026-08-25), once `futureRelease` was set to `30d` there: a submission carrying a `HOLDUNTIL` ten minutes out came back `pending`, with `sendAt` equal to the time asked for and a `250 2.1.5 Queued` from the MTA, rather than going out at once. Worth repeating that the capability is no evidence either way — it advertised `maxDelayedSend: 2592000` and `FUTURERELEASE` while the setting was still off. Only a submission tells you. The rest of the journey is **confirmed live too (2026-08-26)**: a hold expired and was delivered, and the **Scheduled** folder reconciled on the way in — a released message moved to Sent, a cancelled one back to Drafts. Nothing in Stalwart does that moving, so if ihasmail is never opened again the message still goes out; it is only the folder that waits to be tidied. - **Stalwart 0.16 and RFC 8984 disagree about the calendar vocabulary, and the server only says so half the time.** A participant's address lives in `calendarAddress`, not RFC 8984's `sendTo`/`email`; the organizer is `organizerCalendarAddress`, not `replyTo`; and a recurrence is a single `recurrenceRule`, not a `recurrenceRules` array. Addressed the RFC's way, `CalendarEvent/set` **keeps the event and discards the whole participant map without an error** — guests disappeared on save and no invitation was ever sent, which is what [#26](https://github.com/Coffey-Labs/ihasmail/issues/26) reported. The array form of the rule is refused honestly, with `invalidProperties`, so recurring events could not be created at all and existing ones showed no repeat ([#30](https://github.com/Coffey-Labs/ihasmail/issues/30)). ihasmail now writes Stalwart's names and reads either, and the mock refuses what the real server refuses, since advertising the RFC spelling is precisely how this got as far as a live server. Verified against 0.16.19 on 2026-08-25, end to end: participants, organizer and rule all survive a create, an update and a re-read; an invitation to an external Gmail address arrived as an invite card, and the decline came back and was applied to the event (`needs-action` → `declined`, sequence 1). Cancelling the event notified the guest too. Adding guests to an event that had none, and clearing them again with `null`, both work on the update path, as does RSVP — which patches `participants/{key}/participationStatus` (and `participationComment`) rather than sending the whole map. That patch had to be aimed at the base event: through 0.16.19 `CalendarEvent/set` refused a synthetic id with *"Updating synthetic ids is not yet supported"*, which is why RSVP resolves `baseEventId` first. 0.16.20 accepts one, so that resolution is now a choice rather than the only option — an RSVP aimed at an occurrence would answer for that date alone. It still resolves the base, which is the answer people mean. Adding a *new* participant by patch is refused as well (`Patch operation failed`), so a changed guest list is written as the whole `participants` property. One more thing to know when reading this code: an expanded occurrence carries a `recurrenceId` but *no* rule of its own, and `baseEventId` is set on everything an expanded query returns — a one-off included, whose own id differs from its base — so neither is a test for recurrence. -- **Recurring events: colour, category, edit and delete apply to the whole series.** Every entry point resolves `baseEventId` first, and the delete dialog says *"Delete all occurrences?"* out loud. Until 0.16.20 this was the only thing the server allowed: a synthetic id was refused outright. 0.16.20 added `CalendarEvent/set` support for updating and deleting them, writing a `recurrenceOverrides` entry for the occurrence, so editing a single date is now a thing ihasmail could do and does not yet — [#132](https://github.com/Coffey-Labs/ihasmail/issues/132). Worth knowing while it is unbuilt: the refusal that used to catch a synthetic id reaching `destroy` is gone, so an id resolved wrongly now silently deletes one occurrence and reports success rather than erroring. Nothing does that today; [#133](https://github.com/Coffey-Labs/ihasmail/issues/133) moves the resolution to the store so nothing starts. +- **A synthetic id is only true until the next write, and a stale one is wrong rather than invalid.** Stalwart's expanded-occurrence ids encode a position in the series, and writing a `recurrenceOverrides` entry adds a component that renumbers it. **Confirmed live on 0.16.20 (2026-08-31)**: a five-week series came back as `e i m q u` over 03-01 … 03-29; one override written to 03-08 left the *same five ids* addressing 03-01, 03-15, 03-29, 03-08 and 03-22. Nothing was rejected and nothing reported a change — `i` simply meant a week later than it had a moment earlier. So an id cached across a write silently points at another date, and a delete meant for one occurrence removes a different one. This is the second time the same shape of problem has cost a live debugging session, and it is worth saying plainly why it is dangerous: the failure is not a `notFound` a client would notice, it is a confident answer about the wrong day. ihasmail therefore never mutates an occurrence by an id it is holding. `recurrenceId` is the stable name for a slot in a series — it is the date — so `updateEvent` and `destroyEvent` look the current id up by it immediately before they act, and refuse outright if the date is no longer in the series rather than falling back to the id in hand. The mock renumbers too, by a different permutation to the real server's but with the property that matters, since a mock that kept ids stable would agree with precisely the belief that is wrong. + +- **A per-occurrence patch made only of inherited properties creates an override that loses the title.** The twelve properties 0.16.20 drops from a per-occurrence patch are dropped *after* it has decided to write an override, so a patch consisting only of them still writes one — and that override carries the `start` and `duration` the server fills in and nothing else. **Confirmed live on 0.16.20 (2026-08-31)**: `{"privacy": "private"}` aimed at one occurrence answered `updated`, left `privacy` untouched on the series, and left that date with no title at all. A successful response, a silently discarded change, and real data loss on a third property nobody mentioned. ihasmail narrows a per-occurrence patch before sending it and sends nothing when narrowing empties it, which was written as a point of principle — a request whose response could only be a meaningless "updated" is worse than no request — and turns out to prevent this. Worth remembering as the argument for the principle. + +- **Recurring events can be edited and deleted one date at a time, since 0.16.20.** A write aimed at a synthetic id was refused outright through 0.16.19; 0.16.20 turns it into a `recurrenceOverrides` entry instead, so "this occurrence" and "the whole series" are now two different things ihasmail asks about before acting. **Confirmed live on 0.16.20 (2026-08-31)** end to end against a five-week series: a legal patch landed on the override with `start` and `duration` filled in by the server; `useDefaultAlerts` was refused with *"This property cannot be modified on a single occurrence."*; a destroy removed one date and left the series; and a base event and one of its instances in the same request were refused together, both ids, with *"A base event and its instances cannot be modified in the same request."* The scope is chosen before the editor opens rather than on save, because it decides which event the form is about — one populated from the master shows the *series'* start date, so editing Wednesday would have offered to move Monday. Two entries below are the sharp edges this turned up. - Editable date boxes are always Gregorian and in Latin digits, even for locales whose *display* uses another calendar or numbering system (`fa-IR`, `th-TH`, `ar-EG`) — they keep the locale's field order and separator, but a Buddhist-era year in a text box does not round-trip against the Gregorian calendar grid. Non-Gregorian calendar support is not implemented. - The account locale is read from `x:AccountSettings/get`, whose permission the built-in user role has, falling back to `x:Account/get` (which needs the admin-only `sysAccountGet`). Both are Stalwart 0.16 methods: **on older servers neither is reachable** — they do not implement the registry and reject a request that so much as names the `urn:stalwart:jmap` capability — so there the locale still falls back to the browser's and can be chosen by hand. Confirmed live on 0.16.19 (2026-08-25), once the capability was looked for where Stalwart advertises it; a locale request that is merely refused no longer downgrades the detected generation. diff --git a/server/src/mock/index.ts b/server/src/mock/index.ts index 721b603..5fda3a7 100644 --- a/server/src/mock/index.ts +++ b/server/src/mock/index.ts @@ -5,6 +5,7 @@ */ import { createServer, type IncomingMessage, type ServerResponse } from "node:http"; import { randomUUID } from "node:crypto"; +import { expandOccurrences, occurrenceAt, occurrenceView, parseSyntheticId, slotOfOccurrence, splitOccurrencePatch, syntheticId, type Occurrence } from "./recurrence.js"; import { parseOtpauthUrl, verifyTotp } from "../totp.js"; import { holdUntilOf, undoStatusOf } from "./futurerelease.js"; @@ -399,6 +400,24 @@ function genericGet(list: Obj[]) { return { accountId: ACCOUNT, state: String(state.n), list: found.map((x) => pick(x, a.properties as string[] | null)), notFound: ids ? ids.filter((id) => !list.some((x) => x.id === id)) : [] }; }; } +/** + * An id, as either a stored event or one occurrence of one. + * + * A synthetic id whose base is gone, or whose index falls outside the series + * (deleted, or past a `count`), resolves to nothing — `notFound`, the way the + * server answers for an occurrence that is not there any more. + */ +function resolveEvent(list: Obj[], id: string): { base: Obj; occ?: Occurrence } | null { + const direct = list.find((x) => x.id === id); + if (direct) return { base: direct }; + const parsed = parseSyntheticId(id); + if (!parsed) return null; + const base = list.find((x) => x.id === parsed.baseId); + if (!base) return null; + const occ = occurrenceAt(base, parsed.slot); + return occ ? { base, occ } : null; +} + /** Thrown from an onCreate hook to refuse a create the way a real server would. */ class SetError extends Error { constructor(readonly type: string, readonly description: string, readonly properties?: string[]) { super(description); } @@ -436,6 +455,125 @@ function genericSet(list: Obj[], prefix: string, onCreate?: (o: Obj) => void) { }; } +/* ---------- calendar events ---------- */ + +/** + * `CalendarEvent/set`, including the synthetic-id handling 0.16.20 added. + * + * An update or destroy aimed at an occurrence does not touch the series: it + * writes a `recurrenceOverrides` entry keyed by that date, exactly as Stalwart + * does — `{ excluded: true }` for a destroy, the patch merged in for an update. + * + * The refusals are the point of reproducing this at all: + * + * - a base event and one of its instances in the same request is refused, both + * ids at once, because the server cannot apply them in a defined order; + * - the same id twice is "Duplicate event id."; + * - the ten event-level properties are refused with `invalidProperties`; + * - and the twelve inherited ones are dropped in silence, with the response + * still saying the update succeeded. A mock that applied them would let a + * client that sends them look correct everywhere except a real server. + */ +function calendarEventSet(a: Obj) { + const created: Obj = {}; + const updated: Obj = {}; + const destroyed: string[] = []; + const notCreated: Obj = {}; + const notUpdated: Obj = {}; + const notDestroyed: Obj = {}; + + for (const [cid, obj] of Object.entries((a.create as Obj) ?? {})) { + const o: Obj = { ...(obj as Obj), id: `ev${randomUUID().slice(0, 6)}` }; + // Stalwart 0.16 rejects the RFC 8984 array outright and silently discards + // participants addressed the RFC 8984 way. The mock did neither, which is + // how #26 and #30 reached a live server unnoticed — so it does both. + if (o.recurrenceRules) { notCreated[cid] = new SetError("invalidProperties", "Invalid property.", ["recurrenceRules"]).toJSON(); continue; } + const parts = o.participants as Record | undefined; + if (parts && Object.values(parts).some((p) => !p.calendarAddress)) delete o.participants; + if (o.replyTo && !o.organizerCalendarAddress) delete o.replyTo; + o.uid = o.uid ?? randomUUID(); + events.push(o); + created[cid] = { id: o.id }; + } + + const updates = Object.entries((a.update as Obj) ?? {}); + const destroys = ((a.destroy as string[]) ?? []).slice(); + const seen = new Set(); + + /* A base and one of its instances cannot be settled in the same request. */ + const baseOf = (id: string): string | null => { + const r = resolveEvent(events, id); + return r ? (r.base.id as string) : null; + }; + const touched = new Map(); + for (const id of [...updates.map(([id]) => id), ...destroys]) { + const b = baseOf(id); + if (!b) continue; + const entry = touched.get(b) ?? { base: [], instance: [] }; + (parseSyntheticId(id) ? entry.instance : entry.base).push(id); + touched.set(b, entry); + } + const conflicted = new Set(); + for (const [, e] of touched) { + if (e.base.length && e.instance.length) for (const id of [...e.base, ...e.instance]) conflicted.add(id); + } + const conflict = () => new SetError("invalidProperties", "A base event and its instances cannot be modified in the same request.", ["id"]).toJSON(); + + for (const [id, patch] of updates) { + if (conflicted.has(id)) { notUpdated[id] = conflict(); continue; } + if (seen.has(id)) { notUpdated[id] = new SetError("invalidProperties", "Duplicate event id.", ["id"]).toJSON(); continue; } + seen.add(id); + const resolved = resolveEvent(events, id); + if (!resolved) { notUpdated[id] = { type: "notFound" }; continue; } + if (!resolved.occ) { applyPatch(resolved.base, patch as Obj); updated[id] = null; continue; } + const { rejected, applied } = splitOccurrencePatch(patch as Obj); + if (rejected) { notUpdated[id] = new SetError("invalidProperties", "This property cannot be modified on a single occurrence.", [rejected]).toJSON(); continue; } + writeOverride(resolved.base, resolved.occ, applied); + updated[id] = null; + } + + for (const id of destroys) { + if (conflicted.has(id)) { notDestroyed[id] = conflict(); continue; } + const resolved = resolveEvent(events, id); + if (!resolved) { notDestroyed[id] = { type: "notFound" }; continue; } + if (resolved.occ) { + // One date off a series, which is an override rather than a deletion. + writeOverride(resolved.base, resolved.occ, { excluded: true }, true); + destroyed.push(id); + continue; + } + const i = events.findIndex((x) => x.id === id); + if (i >= 0) { events.splice(i, 1); destroyed.push(id); } + } + + return setResp({ + created, updated, destroyed, + ...(Object.keys(notCreated).length ? { notCreated } : {}), + ...(Object.keys(notUpdated).length ? { notUpdated } : {}), + ...(Object.keys(notDestroyed).length ? { notDestroyed } : {}), + }); +} + +/** + * Merge a patch into the override for one date. + * + * Stalwart fills `start` and `duration` in when the patch leaves them out, so + * an override always carries its own timing; the mock does the same, or a + * client could depend on inheriting them and be right only here. + */ +function writeOverride(base: Obj, occ: Occurrence, patch: Obj, replace = false) { + const overrides = (base.recurrenceOverrides as Record | undefined) ?? {}; + const existing = replace ? {} : (overrides[occ.recurrenceId] ?? {}); + const next: Obj = { ...existing }; + if (!replace) { + if (!("start" in next)) next.start = occ.start; + if (!("duration" in next) && base.duration) next.duration = base.duration; + } + applyPatch(next, patch); + overrides[occ.recurrenceId] = next; + base.recurrenceOverrides = overrides; +} + /* ---------- submissions ---------- */ /** * Held messages, the way Stalwart models them: `sendAt` is derived from the @@ -774,18 +912,43 @@ const handlers: Record = { "SieveScript/validate": () => ({ accountId: ACCOUNT, error: null }), "Calendar/get": (a) => hideShareWithUnlessAsked(a, genericGet(calendarsFor(a.accountId))(a) as { list: Obj[] }) as never, "Calendar/set": (a) => genericSet(calendarsFor(a.accountId), "c", (o) => Object.assign(o, { color: "#0f766e", isSubscribed: true, isVisible: true, isDefault: false, includeInAvailability: "all", timeZone: null, shareWith: null, myRights: rightsCal(), description: null, sortOrder: 0, ...o }))(a), - "CalendarEvent/query": (a) => { const list = eventsFor(a.accountId); return { accountId: a.accountId ?? ACCOUNT, queryState: "1", canCalculateChanges: false, position: 0, ids: list.filter((e) => !(a.filter as Obj)?.uid || e.uid === (a.filter as Obj).uid).map((e) => e.id), total: list.length }; }, - "CalendarEvent/get": (a) => genericGet(eventsFor(a.accountId))(a), + /* + * With `expandRecurrences` every id that comes back is synthetic — a one-off + * included, which is what a live 0.16.19 does and what makes `baseEventId` + * useless as a test for a series. Without it (the `findByUid` path) the + * stored ids come back untouched, because callers hand those straight to a + * destroy and mean the whole event. + */ + "CalendarEvent/query": (a) => { + const list = eventsFor(a.accountId); + const filter = (a.filter as Obj) ?? {}; + const matching = list.filter((e) => !filter.uid || e.uid === filter.uid); + if (!a.expandRecurrences) { + return { accountId: a.accountId ?? ACCOUNT, queryState: "1", canCalculateChanges: false, position: 0, ids: matching.map((e) => e.id), total: matching.length }; + } + const from = filter.after ? new Date(filter.after as string) : new Date(-8640000000000); + const to = filter.before ? new Date(filter.before as string) : new Date(8640000000000); + const ids: string[] = []; + for (const e of matching) for (const occ of expandOccurrences(e, from, to)) ids.push(syntheticId(e.id as string, slotOfOccurrence(e, occ))); + return { accountId: a.accountId ?? ACCOUNT, queryState: "1", canCalculateChanges: false, position: 0, ids, total: ids.length }; + }, + "CalendarEvent/get": (a) => { + const list = eventsFor(a.accountId); + const ids = a.ids as string[] | null | undefined; + if (!ids) return genericGet(list)(a); + const found: Obj[] = []; + const notFound: string[] = []; + for (const id of ids) { + const resolved = resolveEvent(list, id); + if (!resolved) { notFound.push(id); continue; } + found.push(resolved.occ ? occurrenceView(resolved.base, resolved.occ) : resolved.base); + } + return { accountId: ACCOUNT, state: String(state.n), list: found.map((x) => pick(x, a.properties as string[] | null)), notFound }; + }, // Stalwart 0.16 rejects the RFC 8984 array outright and silently discards // participants addressed the RFC 8984 way. The mock did neither, which is how // #26 and #30 reached a live server unnoticed — so it now does both. - "CalendarEvent/set": genericSet(events, "ev", (o) => { - if (o.recurrenceRules) throw new SetError("invalidProperties", "Invalid property.", ["recurrenceRules"]); - const parts = o.participants as Record | undefined; - if (parts && Object.values(parts).some((p) => !p.calendarAddress)) delete o.participants; - if (o.replyTo && !o.organizerCalendarAddress) delete o.replyTo; - return Object.assign(o, { uid: o.uid ?? randomUUID() }); - }), + "CalendarEvent/set": (a) => calendarEventSet(a), "CalendarEvent/parse": (a) => { const parsed: Obj = {}; for (const b of a.blobIds as string[]) { const blob = blobs.get(b); if (!blob) continue; const t = blob.data.toString(); const g = (k: string) => new RegExp(`^${k}[^:]*:(.*)$`, "m").exec(t)?.[1]?.trim(); const ds = g("DTSTART") ?? "20260101T000000Z"; const de = g("DTEND") ?? ds; const toLocal = (s: string) => `${s.slice(0, 4)}-${s.slice(4, 6)}-${s.slice(6, 8)}T${s.slice(9, 11)}:${s.slice(11, 13)}:00`; const start = new Date(`${toLocal(ds)}Z`); const end = new Date(`${toLocal(de)}Z`); parsed[b] = { "@type": "Event", uid: g("UID"), title: g("SUMMARY"), start: toLocal(ds), timeZone: "Etc/UTC", duration: `PT${Math.round((end.getTime() - start.getTime()) / 60000)}M`, method: g("METHOD"), locations: g("LOCATION") ? { l: { name: g("LOCATION") } } : undefined, participants: { org: { name: "Ada Lovelace", calendarAddress: "mailto:ada@example.org", roles: { owner: true } }, me: { name: "Demo User", calendarAddress: `mailto:${USER}`, roles: { attendee: true, required: true }, participationStatus: "needs-action" } } }; } return { accountId: ACCOUNT, parsed, notParsable: [] }; }, "ParticipantIdentity/get": genericGet(participantIdentities), "Principal/query": () => ({ accountId: ACCOUNT, queryState: "1", canCalculateChanges: false, position: 0, ids: principals.map((p) => p.id) }), diff --git a/server/src/mock/recurrence.test.ts b/server/src/mock/recurrence.test.ts new file mode 100644 index 0000000..0023f07 --- /dev/null +++ b/server/src/mock/recurrence.test.ts @@ -0,0 +1,168 @@ +import { describe, it } from "node:test"; +import assert from "node:assert/strict"; +import { expandOccurrences, occurrenceAt, occurrenceView, parseSyntheticId, slotOfOccurrence, splitOccurrencePatch, syntheticId } from "./recurrence.js"; + +/** + * The mock expands recurrences so that per-occurrence editing can be developed + * against something. What it has to get right is not the expansion — that is + * the easy half — but the three things a live server does that a client will + * otherwise be written against wrongly: + * + * - every expanded id is synthetic, one-offs included; + * - an occurrence carries a `recurrenceId` and no rule; + * - a per-occurrence patch loses some properties in silence. + */ + +const WEEKDAYS = { "@type": "RecurrenceRule", frequency: "weekly", byDay: [{ day: "mo" }, { day: "tu" }, { day: "we" }, { day: "th" }, { day: "fr" }] }; + +/** A standup at 09:00 every weekday, starting Monday 2026-09-07. */ +const series = () => ({ id: "ev1", "@type": "Event", uid: "u1", title: "Standup", start: "2026-09-07T09:00:00", duration: "PT30M", recurrenceRule: WEEKDAYS } as Record); +const oneOff = () => ({ id: "ev2", "@type": "Event", uid: "u2", title: "Lunch", start: "2026-09-08T12:00:00", duration: "PT1H" } as Record); + +const week = (from: string, to: string) => [new Date(from), new Date(to)] as const; + +describe("expandOccurrences", () => { + it("gives a weekday rule five dates in a week and skips the weekend", () => { + const [a, b] = week("2026-09-07T00:00:00", "2026-09-14T00:00:00"); + const out = expandOccurrences(series(), a, b); + assert.deepEqual(out.map((o) => o.start), [ + "2026-09-07T09:00:00", "2026-09-08T09:00:00", "2026-09-09T09:00:00", + "2026-09-10T09:00:00", "2026-09-11T09:00:00", + ]); + }); + + it("gives a one-off exactly one occurrence, at index 0", () => { + const [a, b] = week("2026-09-01T00:00:00", "2026-10-01T00:00:00"); + const out = expandOccurrences(oneOff(), a, b); + assert.equal(out.length, 1); + assert.equal(out[0]!.index, 0); + }); + + it("honours count", () => { + const ev = { ...series(), recurrenceRule: { ...WEEKDAYS, count: 3 } }; + const [a, b] = week("2026-09-07T00:00:00", "2026-10-01T00:00:00"); + assert.equal(expandOccurrences(ev, a, b).length, 3); + }); + + it("drops an excluded date from the expansion, keeping the series positions", () => { + const ev = { ...series(), recurrenceOverrides: { "2026-09-08T09:00:00": { excluded: true } } }; + const [a, b] = week("2026-09-07T00:00:00", "2026-09-14T00:00:00"); + const out = expandOccurrences(ev, a, b); + assert.deepEqual(out.map((o) => o.start), [ + "2026-09-07T09:00:00", "2026-09-09T09:00:00", "2026-09-10T09:00:00", "2026-09-11T09:00:00", + ]); + // The position within the series is unchanged — Wednesday is still the + // third date the rule produces, whatever happened to Tuesday. It is the + // *id* built on top of that which moves, and only after a write. + assert.equal(out[1]!.index, 2); + }); + + it("carries an override onto the occurrence it keys", () => { + const ev = { ...series(), recurrenceOverrides: { "2026-09-09T09:00:00": { title: "Standup (long)" } } }; + const [a, b] = week("2026-09-07T00:00:00", "2026-09-14T00:00:00"); + const out = expandOccurrences(ev, a, b); + assert.deepEqual(out.find((o) => o.start === "2026-09-09T09:00:00")!.override, { title: "Standup (long)" }); + }); +}); + +describe("occurrenceView", () => { + it("strips the rule, sets recurrenceId, and points baseEventId at the master", () => { + const base = series(); + const occ = occurrenceAt(base, 1)!; + const view = occurrenceView(base, occ); + assert.equal(view.id, syntheticId("ev1", 1)); + assert.equal(view.baseEventId, "ev1"); + assert.equal(view.recurrenceId, "2026-09-08T09:00:00"); + assert.equal(view.recurrenceRule, undefined); + assert.equal(view.recurrenceOverrides, undefined); + }); + + it("gives a one-off a synthetic id over a different base, and no recurrenceId", () => { + // Both halves matter. The id is why `baseEventId` proves nothing about a + // series; the absent `recurrenceId` is why a one-off does not read as one. + const base = oneOff(); + const view = occurrenceView(base, occurrenceAt(base, 0)!); + assert.equal(view.id, "ev2-o0"); + assert.equal(view.baseEventId, "ev2"); + assert.notEqual(view.id, view.baseEventId); + assert.equal(view.recurrenceId, undefined); + }); + + it("lets an override win over the series", () => { + const base = { ...series(), recurrenceOverrides: { "2026-09-08T09:00:00": { title: "Moved" } } }; + // Slot 2, not 1: one override has already shifted the numbering. Reaching + // for the id this occurrence had *before* the write is the bug below. + const view = occurrenceView(base, occurrenceAt(base, 2)!); + assert.equal(view.start, "2026-09-08T09:00:00"); + assert.equal(view.title, "Moved"); + }); +}); + +describe("parseSyntheticId", () => { + it("round-trips", () => { + assert.deepEqual(parseSyntheticId(syntheticId("ev1", 12)), { baseId: "ev1", slot: 12 }); + }); + it("does not claim a stored id", () => { + assert.equal(parseSyntheticId("ev1"), null); + }); +}); + +describe("splitOccurrencePatch", () => { + it("applies what an occurrence takes", () => { + const { rejected, applied } = splitOccurrencePatch({ title: "Just today", color: "#f00" }); + assert.equal(rejected, undefined); + assert.deepEqual(applied, { title: "Just today", color: "#f00" }); + }); + + it("refuses an event-level property by name", () => { + assert.equal(splitOccurrencePatch({ calendarIds: { c2: true } }).rejected, "calendarIds"); + assert.equal(splitOccurrencePatch({ hideAttendees: true }).rejected, "hideAttendees"); + }); + + it("drops an inherited property in silence, which is the dangerous half", () => { + // No `rejected`, nothing applied, and a real server would still answer + // "updated". Anything that trusts the response believes this landed. + const { rejected, applied } = splitOccurrencePatch({ privacy: "private", recurrenceRule: null }); + assert.equal(rejected, undefined); + assert.deepEqual(applied, {}); + }); + + it("judges a pointer patch on its first token", () => { + assert.deepEqual(splitOccurrencePatch({ "participants/me/participationStatus": "accepted" }).applied, + { "participants/me/participationStatus": "accepted" }); + assert.deepEqual(splitOccurrencePatch({ "participants/me/calendarAddress": "mailto:x@y" }).applied, {}); + }); +}); + + +describe("synthetic ids are only true until the next write", () => { + /* + * Confirmed live on 0.16.20 (2026-08-31): writing one `recurrenceOverrides` + * entry renumbered a five-week series so that the *same* ids addressed + * different dates. Nothing was rejected. The mock reproduces the shape of + * that rather than the exact permutation, because the property that bites is + * not which date an id moves to but that it moves at all, silently. + */ + it("makes a cached id address a different date after an override is written", () => { + const before = series(); + const held = syntheticId("ev1", slotOfOccurrence(before, occurrenceAt(before, 3)!)); + const dateBefore = occurrenceAt(before, parseSyntheticId(held)!.slot)!.start; + + const after = { ...before, recurrenceOverrides: { "2026-09-07T09:00:00": { title: "changed" } } }; + const dateAfter = occurrenceAt(after, parseSyntheticId(held)!.slot)!.start; + + assert.notEqual(dateAfter, dateBefore); + // And crucially it still resolves — a stale id is wrong, not invalid, so a + // client that trusts it gets a confident answer about the wrong day. + assert.ok(dateAfter); + }); + + it("keeps recurrenceId meaning the same date across a write, which is why it is the handle", () => { + const before = series(); + const occ = occurrenceAt(before, 3)!; + const after = { ...before, recurrenceOverrides: { "2026-09-07T09:00:00": { title: "changed" } } }; + const same = expandOccurrences(after, new Date("2026-09-01T00:00:00"), new Date("2026-10-01T00:00:00")) + .find((o) => o.recurrenceId === occ.recurrenceId); + assert.equal(same!.start, occ.start); + }); +}); diff --git a/server/src/mock/recurrence.ts b/server/src/mock/recurrence.ts new file mode 100644 index 0000000..2ce75c4 --- /dev/null +++ b/server/src/mock/recurrence.ts @@ -0,0 +1,224 @@ +/** + * Enough recurrence expansion for the mock to behave like Stalwart 0.16.20. + * + * The mock used to hand a recurring event back once, as its stored self. Three + * things that only a live server showed were therefore impossible to develop + * against, and all three had already cost a debugging session: + * + * - an expanded query gives *everything* a synthetic id over a `baseEventId`, + * a one-off included, so `baseEventId` is no evidence of a series; + * - an occurrence carries a `recurrenceId` and no rule of its own; + * - 0.16.20 takes a write aimed at a synthetic id and turns it into a + * `recurrenceOverrides` entry rather than touching the series. + * + * A mock that agrees with the client rather than with the server is how #26 and + * #30 reached a live instance, so the refusals matter as much as the successes: + * what Stalwart rejects is rejected here, and what it drops in silence is + * dropped here, in silence, on purpose. + */ + +export type Obj = Record; + +/** How far the expander will walk before giving up on a rule. */ +const MAX_ITERATIONS = 750; + +const DAYS = ["su", "mo", "tu", "we", "th", "fr", "sa"]; + +/** + * The id an occurrence is addressed by, which is only true until the next write. + * + * Stalwart's are opaque; the mock's are parseable because it has to resolve + * them, and nothing in ihasmail may read either. + * + * They are also deliberately **unstable**, because the real ones are. + * **Confirmed live on 0.16.20 (2026-08-31):** a synthetic id encodes a position + * in the expanded series, and writing a `recurrenceOverrides` entry adds a + * component that renumbers it. A five-week series held `e i m q u` over + * 03-01…03-29; after one override was written to 03-08 the same ids addressed + * 03-01, 03-15, 03-29, 03-08, 03-22. Nothing was rejected — they just meant + * different dates. + * + * That is the hazard worth reproducing, and note which way round it goes: a + * stale id is not *invalid*, it is *wrong*. A mock that expired them instead + * would hand back a loud `notFound` and let a client that caches ids look + * careful. So the numbering is shifted by the number of overrides — an + * arbitrary stand-in for Stalwart's renumbering, with the one property that + * matters: hold an id across a write and it silently addresses another date. + */ +export const syntheticId = (baseId: string, slot: number): string => `${baseId}-o${slot}`; + +export function parseSyntheticId(id: string): { baseId: string; slot: number } | null { + const m = /^(.+)-o(\d+)$/.exec(id); + return m ? { baseId: m[1]!, slot: Number(m[2]) } : null; +} + +/** How far the id numbering has been rotated away from the series order. */ +function rotation(base: Obj): number { + return Object.keys((base.recurrenceOverrides as Record | undefined) ?? {}).length; +} + +/** The id slot this occurrence currently answers to. */ +export function slotOfOccurrence(base: Obj, occ: Occurrence): number { + return occ.index + rotation(base); +} + +/** `2026-08-31T09:00:00` — the naive local form the mock stores `start` in. */ +export function localDateTime(d: Date): string { + const p = (n: number) => String(n).padStart(2, "0"); + return `${d.getFullYear()}-${p(d.getMonth() + 1)}-${p(d.getDate())}T${p(d.getHours())}:${p(d.getMinutes())}:${p(d.getSeconds())}`; +} + +const parseLocal = (s: string): Date => new Date(s); + +export interface Occurrence { + index: number; + /** The slot in the series this instance fills, which keys any override. */ + recurrenceId: string; + start: string; + /** Set when a `recurrenceOverrides` entry applies to this date. */ + override?: Obj; +} + +interface Rule { + frequency?: string; + interval?: number; + count?: number; + until?: string; + byDay?: { day: string }[]; +} + +/** + * Every occurrence of `base` between `from` and `to`, in series order. + * + * An event with no rule has exactly one, at index 0 — which is what gives a + * one-off the synthetic id a real server would give it. + */ +export function expandOccurrences(base: Obj, from: Date, to: Date): Occurrence[] { + const overrides = (base.recurrenceOverrides as Record | undefined) ?? {}; + const startStr = base.start as string; + if (!startStr) return []; + const first = parseLocal(startStr); + const rule = base.recurrenceRule as Rule | undefined; + + const out: Occurrence[] = []; + const emit = (index: number, at: Date): boolean => { + const recurrenceId = localDateTime(at); + const override = overrides[recurrenceId]; + // An excluded date is simply gone from the expansion. Its slot is not + // reserved -- see `syntheticId` for why nothing here pretends otherwise. + if (override?.excluded === true) return true; + if (at >= from && at < to) { + out.push({ index, recurrenceId, start: recurrenceId, ...(override ? { override } : {}) }); + } + return at < to; + }; + + if (!rule?.frequency) { + emit(0, first); + return out; + } + + const interval = Math.max(1, rule.interval ?? 1); + const until = rule.until ? parseLocal(rule.until) : null; + const byDay = rule.byDay?.length ? new Set(rule.byDay.map((d) => d.day.toLowerCase())) : null; + + let index = 0; + let emitted = 0; + const cursor = new Date(first); + + for (let step = 0; step < MAX_ITERATIONS; step++) { + if (until && cursor > until) break; + if (rule.count != null && emitted >= rule.count) break; + + const matches = !byDay || byDay.has(DAYS[cursor.getDay()]!); + if (matches) { + emitted++; + const keepGoing = emit(index, new Date(cursor)); + index++; + if (!keepGoing) break; + } + + // A rule with byDay walks day by day and keeps the days it names; without + // one it steps by its own frequency. + if (byDay) cursor.setDate(cursor.getDate() + 1); + else if (rule.frequency === "daily") cursor.setDate(cursor.getDate() + interval); + else if (rule.frequency === "weekly") cursor.setDate(cursor.getDate() + 7 * interval); + else if (rule.frequency === "monthly") cursor.setMonth(cursor.getMonth() + interval); + else if (rule.frequency === "yearly") cursor.setFullYear(cursor.getFullYear() + interval); + else break; + } + return out; +} + +/** Fields that describe the series and never travel down to one instance. */ +const SERIES_ONLY = ["recurrenceRule", "recurrenceRules", "excludedRecurrenceRules", "recurrenceOverrides"]; + +/** + * The object a `CalendarEvent/get` returns for one occurrence. + * + * The rule is stripped, `recurrenceId` is set, and `baseEventId` points at the + * master — so an occurrence is recognisable by its `recurrenceId` and by + * nothing else, which is the shape `isRecurring` was written against. + */ +export function occurrenceView(base: Obj, occ: Occurrence): Obj { + const view: Obj = { ...base }; + for (const k of SERIES_ONLY) delete view[k]; + Object.assign(view, occ.override ?? {}); + view.id = syntheticId(base.id as string, slotOfOccurrence(base, occ)); + view.baseEventId = base.id; + view.start = occ.start; + // Only a genuine instance of a series carries one. A one-off expanded into + // its single occurrence does not, or every one-off would look recurring. + if (base.recurrenceRule) view.recurrenceId = occ.recurrenceId; + delete view.excluded; + return view; +} + +/* ---------- what a single occurrence will not take ---------- */ + +/** Refused outright, with `invalidProperties`. */ +export const OCCURRENCE_REJECTED = new Set([ + "baseEventId", "calendarIds", "isDraft", "isOrigin", "utcStart", "utcEnd", + "useDefaultAlerts", "mayInviteSelf", "mayInviteOthers", "hideAttendees", +]); + +/** + * Dropped from the patch, with the response still reporting success. + * + * This is the half that has to be reproduced most carefully. A mock that + * *applied* these would agree with a client that sends them, and the belief + * would ship — which is exactly the road #26 took to a live server. + */ +export const OCCURRENCE_INHERITED = new Set([ + "@type", "method", "organizerCalendarAddress", "privacy", "prodId", + "recurrenceId", "recurrenceIdTimeZone", "sentBy", "uid", + "recurrenceOverrides", "recurrenceRule", "relatedTo", +]); + +/** + * Split a per-occurrence patch the way the server's validator does. + * + * `rejected` is the first property that would be refused, if any; `applied` is + * what actually lands on the override. Everything else vanishes without a word. + */ +export function splitOccurrencePatch(patch: Obj): { rejected?: string; applied: Obj } { + const applied: Obj = {}; + for (const [key, value] of Object.entries(patch)) { + const [head, , third] = key.split("/"); + const root = head ?? key; + if (OCCURRENCE_REJECTED.has(root)) return { rejected: root, applied }; + if (OCCURRENCE_INHERITED.has(root)) continue; + if (root === "participants" && third === "calendarAddress") continue; + if (root === "id") continue; + applied[key] = value; + } + return { applied }; +} + +/** The occurrence a slot currently addresses — which is not a fixed thing. */ +export function occurrenceAt(base: Obj, slot: number): Occurrence | null { + const index = slot - rotation(base); + if (index < 0) return null; + const all = expandOccurrences(base, new Date(-8640000000000), new Date(8640000000000)); + return all.find((o) => o.index === index) ?? null; +} diff --git a/web/src/store/__tests__/event-scope.test.ts b/web/src/store/__tests__/event-scope.test.ts index 70f5ae8..e2e0b22 100644 --- a/web/src/store/__tests__/event-scope.test.ts +++ b/web/src/store/__tests__/event-scope.test.ts @@ -1,6 +1,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { CAP, client } from "@/jmap/client"; -import { eventIdForScope, isOccurrence, useCalendar } from "@/store/calendar"; +import { CalendarSetError, eventIdForScope, isOccurrence, isThisAndFutureRefusal, occurrencePatch, OccurrenceScopeError, useCalendar } from "@/store/calendar"; import type { CalendarEvent, JmapSession } from "@/jmap/types"; /** @@ -38,11 +38,25 @@ const MASTER: CalendarEvent = { ...OCCURRENCE, id: "i", baseEventId: undefined, interface SetCall { update?: Record; destroy?: string[] } -function server() { +/** + * A server that renumbers, the way 0.16.20 does. + * + * `resolvesTo` is the id the occurrence answers to *now* — deliberately not the + * id the cached object carries, because that is exactly the situation a write + * to the series leaves behind. A store that sends the id it was handed rather + * than the one it looked up will send `iaaaaas` and these tests will say so. + */ +function server(opts: { resolvesTo?: string | null } = {}) { const calls: SetCall[] = []; + const resolved = opts.resolvesTo === undefined ? OCCURRENCE.id : opts.resolvesTo; const fetchMock = vi.fn(async (_url: string, init: RequestInit) => { const body = JSON.parse(init.body as string) as { methodCalls: [string, Record, string][] }; const methodResponses = body.methodCalls.map(([name, args, id]) => { + if (name === "CalendarEvent/get" && id === "g") { + // The re-resolution lookup: same recurrenceId, whatever id it wears now. + const list = resolved ? [{ ...OCCURRENCE, id: resolved }] : []; + return [name, { accountId: "a1", state: "1", list, notFound: [] }, id]; + } if (name === "CalendarEvent/set") { calls.push({ update: args.update as Record, destroy: args.destroy as string[] }); return [name, { @@ -115,10 +129,21 @@ describe("destroyEvent", () => { expect(calls[0]!.destroy).not.toContain("iaaaaas"); }); - it("sends the synthetic id for a single occurrence", async () => { - const calls = server(); + it("sends the id the occurrence answers to now, not the one it was handed", async () => { + // The live finding: writing one override renumbers the series, so an id + // cached a moment ago addresses a different date. `recurrenceId` is the + // stable handle, so the store looks the current id up by it. + const calls = server({ resolvesTo: "renumbered7" }); await useCalendar.getState().destroyEvent(OCCURRENCE, false, "occurrence"); - expect(calls[0]!.destroy).toEqual(["iaaaaas"]); + expect(calls[0]!.destroy).toEqual(["renumbered7"]); + expect(calls[0]!.destroy).not.toContain("iaaaaas"); + }); + + it("refuses rather than guessing when the date is no longer in the series", async () => { + const calls = server({ resolvesTo: null }); + await expect(useCalendar.getState().destroyEvent(OCCURRENCE, false, "occurrence")) + .rejects.toThrow(/no longer part of this series/i); + expect(calls).toEqual([]); }); it("drops the occurrence from the cache without evicting the master", async () => { @@ -137,10 +162,10 @@ describe("updateEvent", () => { expect(Object.keys(calls[0]!.update!)).toEqual(["i"]); }); - it("patches the instance for a single occurrence", async () => { - const calls = server(); + it("patches the id the occurrence answers to now", async () => { + const calls = server({ resolvesTo: "renumbered7" }); await useCalendar.getState().updateEvent(OCCURRENCE, { color: "#f00" }, false, "occurrence"); - expect(Object.keys(calls[0]!.update!)).toEqual(["iaaaaas"]); + expect(Object.keys(calls[0]!.update!)).toEqual(["renumbered7"]); }); }); @@ -161,3 +186,71 @@ describe("rsvp", () => { await expect(useCalendar.getState().rsvp(OCCURRENCE, "accepted")).rejects.toThrow(/not a participant/i); }); }); + + +describe("occurrencePatch", () => { + it("lets through what one date will actually take", () => { + const { patch, dropped } = occurrencePatch({ title: "Just today", color: "#f00" }); + expect(patch).toEqual({ title: "Just today", color: "#f00" }); + expect(dropped).toEqual([]); + }); + + it("throws on a property the server refuses outright", () => { + // Loud is correct here: moving one occurrence to another calendar is not + // something the user can be quietly given a different answer to. + expect(() => occurrencePatch({ calendarIds: { c2: true } })).toThrow(OccurrenceScopeError); + expect(() => occurrencePatch({ useDefaultAlerts: false })).toThrow(/whole series/i); + }); + + it("removes an inherited property and reports it, rather than letting it vanish", () => { + // The server would take this patch, drop `privacy`, and answer "updated". + // Anything that believes the response believes the change landed. + const { patch, dropped } = occurrencePatch({ title: "x", privacy: "private", recurrenceRule: null }); + expect(patch).toEqual({ title: "x" }); + expect(dropped).toEqual(["privacy", "recurrenceRule"]); + }); + + it("judges a pointer patch on its first token, as the server does", () => { + expect(occurrencePatch({ "participants/me/participationStatus": "accepted" }).patch) + .toEqual({ "participants/me/participationStatus": "accepted" }); + expect(occurrencePatch({ "participants/me/calendarAddress": "mailto:x@y" }).dropped) + .toEqual(["participants/me/calendarAddress"]); + }); +}); + +describe("updateEvent, per occurrence", () => { + it("narrows the patch before sending it and reports what it kept back", async () => { + const calls = server(); + const dropped = await useCalendar.getState().updateEvent(OCCURRENCE, { title: "Just today", privacy: "private" }, false, "occurrence"); + expect(calls[0]!.update).toEqual({ iaaaaas: { title: "Just today" } }); + expect(dropped).toEqual(["privacy"]); + }); + + it("sends nothing at all when a patch is entirely inherited", async () => { + // A request that could only be a no-op is worse than no request: the + // response would say "updated" and mean nothing by it. + const calls = server(); + const dropped = await useCalendar.getState().updateEvent(OCCURRENCE, { privacy: "private" }, false, "occurrence"); + expect(calls).toEqual([]); + expect(dropped).toEqual(["privacy"]); + }); + + it("leaves a series patch exactly as the caller wrote it", async () => { + const calls = server(); + await useCalendar.getState().updateEvent(OCCURRENCE, { privacy: "private", useDefaultAlerts: false }, false, "series"); + expect(calls[0]!.update!.i).toEqual({ privacy: "private", useDefaultAlerts: false }); + }); +}); + +describe("isThisAndFutureRefusal", () => { + it("recognises the refusal worth offering the series for", () => { + expect(isThisAndFutureRefusal(new CalendarSetError({ + type: "invalidProperties", + description: "Occurrences of a this-and-future change cannot be modified individually.", + }))).toBe(true); + }); + it("does not claim an unrelated refusal", () => { + expect(isThisAndFutureRefusal(new CalendarSetError({ type: "forbidden", description: "Nope." }))).toBe(false); + expect(isThisAndFutureRefusal(new Error("Occurrences of a this-and-future change"))).toBe(false); + }); +}); diff --git a/web/src/store/calendar.ts b/web/src/store/calendar.ts index 2ad1f5b..bb0a507 100644 --- a/web/src/store/calendar.ts +++ b/web/src/store/calendar.ts @@ -83,6 +83,137 @@ export function isOccurrence(event: CalendarEvent): boolean { return event.baseEventId != null && event.baseEventId !== event.id; } +/** + * What `CalendarEvent/set` will not take on a single occurrence, and why the + * client has to know rather than letting the server sort it out. + * + * 0.16.20's per-occurrence validator sorts properties into three groups, and + * only one of them is honest about itself: + * + * - **Rejected** — `invalidProperties`, *"This property cannot be modified on a + * single occurrence."* Loud, and fine. + * - **Inherited** — dropped from the patch, and the response still says the + * update succeeded. Nothing anywhere reports it. + * - Everything else, which is applied to the override. + * + * The middle group is the whole problem. It is the same failure as [#26], where + * a participant map addressed the RFC 8984 way was discarded without an error + * and the client showed the guests as saved: a successful response is not + * evidence that anything was written. So a per-occurrence patch is checked here + * before it is sent — rejected properties throw, inherited ones are reported to + * the caller — rather than being posted hopefully and believed. + * + * [#26]: https://github.com/Coffey-Labs/ihasmail/issues/26 + */ +const OCCURRENCE_REJECTED = new Set([ + "baseEventId", "calendarIds", "isDraft", "isOrigin", "utcStart", "utcEnd", + "useDefaultAlerts", "mayInviteSelf", "mayInviteOthers", "hideAttendees", +]); + +/** Applied to the series and never to one date; dropped in silence if sent. */ +const OCCURRENCE_INHERITED = new Set([ + "@type", "method", "organizerCalendarAddress", "privacy", "prodId", + "recurrenceId", "recurrenceIdTimeZone", "sentBy", "uid", + "recurrenceOverrides", "recurrenceRule", "relatedTo", +]); + +/** + * A `notUpdated`/`notDestroyed` entry, kept whole rather than flattened. + * + * Some refusals are worth acting on rather than only showing: 0.16.20 will not + * edit an occurrence that belongs to a this-and-future change, and the useful + * response to that is to offer the series, which needs the reason and not just + * its text. + */ +export class CalendarSetError extends Error { + constructor(readonly setError: { type: string; description?: string; properties?: string[] }) { + super(setErrorMessage(setError)); + this.name = "CalendarSetError"; + } +} + +/** Whether a refusal was "this occurrence belongs to a this-and-future change". */ +export function isThisAndFutureRefusal(err: unknown): boolean { + return err instanceof CalendarSetError && /this-and-future/i.test(err.setError.description ?? ""); +} + +/** + * A synthetic id is only true until the next write, so an occurrence is + * re-resolved from its `recurrenceId` immediately before it is touched. + * + * **Confirmed live on 0.16.20 (2026-08-31.)** Stalwart's synthetic ids encode a + * position in the expanded series, and writing a `recurrenceOverrides` entry + * adds a component that renumbers it. A five-week series held ids `e i m q u` + * over 03-01…03-29; after one override was written to 03-08 the *same ids* + * addressed 03-01, 03-15, 03-29, 03-08, 03-22. Not one of them was rejected — + * `i` simply meant a week later than it had a moment before. + * + * So an id cached across a write silently points at a different date, and a + * delete aimed at one occurrence removes another. `recurrenceId` is the stable + * name for a slot in a series — it is the date itself — so that is what we hold + * and what we look the current id up by. + */ +async function currentOccurrenceId(accountId: Id, event: CalendarEvent): Promise { + const base = event.baseEventId; + const rid = event.recurrenceId; + // A one-off, or an object with nothing to re-resolve from: its own id is all + // there is, and there is no series for a write to have renumbered. + if (!base || !rid) return event.id; + + const around = new Date(rid); + if (Number.isNaN(around.getTime())) return event.id; + const from = new Date(around.getTime() - DAY_MS); + const to = new Date(around.getTime() + DAY_MS); + + const res = await client.chain([ + ["CalendarEvent/query", { accountId, filter: { after: toLocalDateTime(from), before: toLocalDateTime(to) }, expandRecurrences: true, limit: 200 }, "q"], + ["CalendarEvent/get", { accountId, "#ids": { resultOf: "q", name: "CalendarEvent/query", path: "/ids" }, properties: ["id", "baseEventId", "recurrenceId"] }, "g"], + ]); + const list = (res.get("g")?.[0] as unknown as GetResponse | undefined)?.list ?? []; + const found = list.find((e) => e.baseEventId === base && e.recurrenceId === rid); + if (!found) { + // The date is gone -- already excluded, or the series no longer reaches it. + // Better to say so than to act on an id that means something else now. + throw new Error("That occurrence is no longer part of this series. Reload the calendar and try again."); + } + return found.id; +} + +export class OccurrenceScopeError extends Error { + constructor(readonly property: string) { + super(`"${property}" applies to the whole series and cannot be changed for one occurrence.`); + this.name = "OccurrenceScopeError"; + } +} + +/** + * A patch narrowed to what one occurrence will actually accept. + * + * Throws `OccurrenceScopeError` on a property the server would refuse, and + * returns the inherited ones it removed so a caller can say what it could not + * do for this date alone instead of claiming it did. + * + * Patch *pointers* are judged on their first token, the way the server does: + * `participants/{key}/participationStatus` is allowed, and + * `participants/{key}/calendarAddress` is one of the silent drops. + */ +export function occurrencePatch(patch: Record): { patch: Record; dropped: string[] } { + const out: Record = {}; + const dropped: string[] = []; + for (const [key, value] of Object.entries(patch)) { + const [head, , third] = key.split("/"); + const root = head ?? key; + if (OCCURRENCE_REJECTED.has(root)) throw new OccurrenceScopeError(root); + if (OCCURRENCE_INHERITED.has(root)) { dropped.push(root); continue; } + if (root === "participants" && third === "calendarAddress") { dropped.push(key); continue; } + // `id` is immutable; the server errors on a value that is not the event's + // own, and ignores one that is. Neither is worth sending. + if (root === "id") { dropped.push(root); continue; } + out[key] = value; + } + return { patch: out, dropped }; +} + /** A calendar somebody else shared, and the account it lives in. */ export interface SharedCalendar { accountId: Id; @@ -122,7 +253,8 @@ interface CalendarState { instancesIn(start: Date, end: Date): EventInstance[]; getEvent(id: Id): Promise; createEvent(event: Partial, calendarId: Id, sendInvites: boolean): Promise; - updateEvent(event: CalendarEvent, patch: Record, sendInvites: boolean, scope: EventScope): Promise; + /** Returns the properties that had to be left to the series, if any. */ + updateEvent(event: CalendarEvent, patch: Record, sendInvites: boolean, scope: EventScope): Promise; destroyEvent(event: CalendarEvent, sendInvites: boolean, scope: EventScope): Promise; rsvp(event: CalendarEvent, status: "accepted" | "tentative" | "declined", comment?: string): Promise; createCalendar(data: Partial): Promise; @@ -398,19 +530,24 @@ export const useCalendar = create((set, get) => ({ async updateEvent(event, patch, sendInvites, scope) { const accountId = get().accountId!; - const id = eventIdForScope(event, scope); - const res = await client.call("CalendarEvent/set", { accountId, update: { [id]: patch }, sendSchedulingMessages: sendInvites }); + const id = scope === "occurrence" ? await currentOccurrenceId(accountId, event) : eventIdForScope(event, scope); + // An occurrence takes less than the series does, and says so about only + // half of it. Narrow the patch here rather than posting it hopefully. + const { patch: body, dropped } = scope === "occurrence" ? occurrencePatch(patch) : { patch, dropped: [] as string[] }; + if (!Object.keys(body).length) return dropped; + const res = await client.call("CalendarEvent/set", { accountId, update: { [id]: body }, sendSchedulingMessages: sendInvites }); const err = res.notUpdated?.[id]; - if (err) throw new Error(setErrorMessage(err)); + if (err) throw new CalendarSetError(err); get().invalidate(); + return dropped; }, async destroyEvent(event, sendInvites, scope) { const accountId = get().accountId!; - const id = eventIdForScope(event, scope); + const id = scope === "occurrence" ? await currentOccurrenceId(accountId, event) : eventIdForScope(event, scope); const res = await client.call("CalendarEvent/set", { accountId, destroy: [id], sendSchedulingMessages: sendInvites }); const err = res.notDestroyed?.[id]; - if (err) throw new Error(setErrorMessage(err)); + if (err) throw new CalendarSetError(err); set((s) => { const events = { ...s.events }; // Drop both ids: the one that was sent, and the object as the caller diff --git a/web/src/styles/app.css b/web/src/styles/app.css index f3cbd4d..e61907e 100644 --- a/web/src/styles/app.css +++ b/web/src/styles/app.css @@ -310,6 +310,11 @@ a.menu-item:hover { color: var(--fg); } .dialog-body { padding: 8px 20px 16px; overflow: auto; } .dialog-foot { display: flex; align-items: center; justify-content: flex-end; gap: 8px; padding: 12px 20px 16px; border-top: 1px solid var(--border); } .dialog-foot .left { margin-right: auto; } +/* "This occurrence or the whole series" — one button per answer, stacked, so + the destructive one is read rather than landed on by muscle memory. */ +.dialog-choices { display: flex; flex-direction: column; gap: 8px; } +.dialog-choice { display: flex; flex-direction: column; align-items: flex-start; gap: 2px; width: 100%; text-align: left; padding: 10px 12px; height: auto; } +.dialog-choice small { font-weight: 400; opacity: 0.75; } /* Toasts ----------------------------------------------------------------- */ .toast-host { position: fixed; left: 50%; bottom: 24px; transform: translateX(-50%); z-index: 3000; display: flex; flex-direction: column; gap: 8px; align-items: center; pointer-events: none; padding: 0 12px; width: 100%; max-width: 520px; } diff --git a/web/src/ui/dialog.tsx b/web/src/ui/dialog.tsx index b746d77..c323ca6 100644 --- a/web/src/ui/dialog.tsx +++ b/web/src/ui/dialog.tsx @@ -86,9 +86,17 @@ export function Dialog({ open, onClose, title, children, footer, size = "md", cl /* ---------- Imperative confirm / prompt ---------- */ +export interface DialogChoice { + value: string; + label: string; + /** Shown under the label, for the choice that needs the caveat. */ + hint?: string; + danger?: boolean; +} + interface ConfirmRequest { id: number; - kind: "confirm" | "prompt"; + kind: "confirm" | "prompt" | "choice"; title: string; message?: ReactNode; confirmLabel?: string; @@ -96,6 +104,7 @@ interface ConfirmRequest { danger?: boolean; defaultValue?: string; placeholder?: string; + choices?: DialogChoice[]; resolve: (v: boolean | string | null) => void; } @@ -119,6 +128,18 @@ export function promptDialog(opts: { title: string; message?: ReactNode; default }); } +/** + * A question with more than two answers, which "this one or all of them" is. + * + * Resolves to the chosen `value`, or `null` if the dialog is dismissed — + * dismissing is not one of the choices, so a caller cannot mistake it for one. + */ +export function choiceDialog(opts: { title: string; message?: ReactNode; choices: DialogChoice[]; cancelLabel?: string }): Promise { + return new Promise((resolve) => { + useConfirmStore.getState().push({ id: reqId++, kind: "choice", ...opts, resolve: (v) => resolve(typeof v === "string" ? v : null) }); + }); +} + export function ConfirmHost() { const req = useConfirmStore((s) => s.queue[0]); const pop = useConfirmStore((s) => s.pop); @@ -132,21 +153,37 @@ export function ConfirmHost() { return ( done(req.kind === "prompt" ? null : false)} + onClose={() => done(req.kind === "confirm" ? false : null)} title={req.title} size="sm" footer={ - <> - - - + ) : ( + <> + + + + ) } > {req.message &&

{req.message}

} + {req.kind === "choice" && ( +
+ {req.choices?.map((c) => ( + + ))} +
+ )} {req.kind === "prompt" && (
{ diff --git a/web/src/views/calendar/CalendarContextMenu.tsx b/web/src/views/calendar/CalendarContextMenu.tsx index d9c8b08..20827d1 100644 --- a/web/src/views/calendar/CalendarContextMenu.tsx +++ b/web/src/views/calendar/CalendarContextMenu.tsx @@ -1,13 +1,13 @@ import { Calendar as CalIcon, CalendarDays, Copy, ExternalLink, Palette, Pencil, Plus, Tag, Trash2, X } from "lucide-react"; import { useLocation } from "wouter"; import type { CalendarEvent } from "@/jmap/types"; -import { useCalendar, isRecurring, type EventInstance } from "@/store/calendar"; +import { useCalendar, isRecurring, isOccurrence, type EventInstance, type EventScope } from "@/store/calendar"; import { useSettings } from "@/store/settings"; import { formatDayMonth } from "@/lib/datetime"; import { MenuItem, MenuSep, MenuTitle, Popover, type Anchor } from "@/ui/popover"; -import { CALENDAR_COLORS } from "@/ui/misc"; import { confirmDialog } from "@/ui/dialog"; import { toast } from "@/ui/toast"; +import { askDeleteScope, askEditScope, droppedMessage, runScoped } from "./scope"; import { toLocalDateOnly } from "@/lib/dates"; import { formatTime } from "@/lib/format"; @@ -65,14 +65,18 @@ export function CalendarContextMenu({ ctx, onClose, onOpen, onEdit, onCreate }: const participants = Object.keys(ev.participants ?? {}).length; const patch = async (p: Record, msg: string) => { + const scope = await askEditScope(ev); + if (!scope) return; try { - await cal.updateEvent(ev, p, false, "series"); - toast.success(msg); + const dropped = await runScoped(scope, (s) => cal.updateEvent(ev, p, false, s)); + if (!dropped) return; + // A per-occurrence change can be accepted in part. Say which part. + toast.success(droppedMessage(dropped) ?? (scope === "occurrence" ? `${msg} for this date` : msg)); } catch (err) { toast.error((err as Error).message); } }; - const setColor = (color: string | null) => void patch({ color }, color ? "Colour updated" : "Colour reset"); + const setColor = (color: string | null) => void patch({ color }, color ? "Colour updated" : "Custom colour removed"); const setCategory = (cat: { name: string; color: string } | null) => { const categoriesPatch = cat ? { [cat.name]: true } : null; void patch({ categories: categoriesPatch, color: cat ? cat.color : null }, cat ? `Categorised as ${cat.name}` : "Category cleared"); @@ -88,11 +92,16 @@ export function CalendarContextMenu({ ctx, onClose, onOpen, onEdit, onCreate }: }; const del = async () => { onClose(); - const recurring = isRecurring(ev); - if (!(await confirmDialog({ title: recurring ? "Delete all occurrences?" : "Delete this event?", confirmLabel: "Delete", danger: true }))) return; + let scope: EventScope | null = "series"; + if (isRecurring(ev) && isOccurrence(ev)) { + scope = await askDeleteScope(ev); + } else if (!(await confirmDialog({ title: "Delete this event?", confirmLabel: "Delete", danger: true }))) { + scope = null; + } + if (!scope) return; try { - await cal.destroyEvent(ev, participants > 1, "series"); - toast.success("Event deleted"); + await runScoped(scope, (s) => cal.destroyEvent(ev, participants > 1, s)); + toast.success(scope === "occurrence" ? "Occurrence deleted" : "Event deleted"); } catch (err) { toast.error((err as Error).message); } @@ -112,14 +121,17 @@ export function CalendarContextMenu({ ctx, onClose, onOpen, onEdit, onCreate }: ))} } label="No category" disabled={!currentCat} onClick={() => { onClose(); setCategory(null); }} /> } label="Manage categories…" onClick={() => { onClose(); navigate("/settings/calendar"); }} /> - - Colour -
- {CALENDAR_COLORS.map((c) => ( -
- {ev.color && } label="Use calendar colour" onClick={() => { onClose(); setColor(null); }} />} + {/* + A colour is what a category already carries, so a second way to set + one just made two things that could disagree. Picking a category is + now the only way to colour an event here. + + Clearing one stays, though, and only when there is one to clear: an + event that already has an explicit colour — set before this, or by + another client — would otherwise ignore its category for ever with + nothing on the menu to say why. + */} + {ev.color && } label="Clear custom colour" onClick={() => { onClose(); setColor(null); }} />} } label="Delete" onClick={() => void del()} /> diff --git a/web/src/views/calendar/EventEditor.tsx b/web/src/views/calendar/EventEditor.tsx index 91133bb..2e0d333 100644 --- a/web/src/views/calendar/EventEditor.tsx +++ b/web/src/views/calendar/EventEditor.tsx @@ -1,7 +1,7 @@ -import { useEffect, useMemo, useState } from "react"; +import { useEffect, useMemo, useRef, useState } from "react"; import { Plus, Trash2, Users } from "lucide-react"; import type { BusyPeriod, CalendarEvent, EmailAddress, JSCalendarAlert, JSCalendarParticipant, JSCalendarRecurrenceRule, JSCalendarNDay } from "@/jmap/types"; -import { useCalendar, myParticipantKeys, isRecurring, eventRule, makeParticipant, participantEmail } from "@/store/calendar"; +import { useCalendar, myParticipantKeys, isRecurring, isOccurrence, eventRule, makeParticipant, participantEmail, type EventScope } from "@/store/calendar"; import { useSettings } from "@/store/settings"; import { useSession } from "@/store/session"; import { useContacts } from "@/store/contacts"; @@ -14,6 +14,7 @@ import { browserTimeZone, dateToZonedLocal, formatDuration, fromInputDateTime, l import { formatClock, formatNumericDate, formatWeekday } from "@/lib/datetime"; import { WEEKDAYS, describeRule, presetFor, ruleFromPreset, type RecurrencePreset } from "@/lib/recurrence"; import { newKey } from "@/lib/contacts"; +import { askEditScope, droppedMessage, runScoped } from "./scope"; export interface EditorInit { event?: CalendarEvent; @@ -24,25 +25,68 @@ export interface EditorInit { const ALERT_OPTIONS = [0, 5, 10, 15, 30, 60, 120, 1440, 2880, 10080]; +/** + * Fields this form always sends that a single occurrence will not take. + * + * `useDefaultAlerts` and `calendarIds` are refused with `invalidProperties`; + * the rest are dropped from the patch while the response still reports + * success. Both halves are reasons not to send them — the second more so, + * because nothing would say it had happened. + */ +const OCCURRENCE_OMIT = new Set(["useDefaultAlerts", "calendarIds", "recurrenceRule", "privacy", "organizerCalendarAddress"]); + export function EventEditor({ init, onClose }: { init: EditorInit; onClose: () => void }) { const cal = useCalendar(); const settings = useSettings((s) => s.settings); const session = useSession((s) => s.session); const [base, setBase] = useState(init.event && !init.event.baseEventId ? init.event : undefined); + const [scope, setScope] = useState(init.event?.baseEventId ? undefined : "series"); const editing = Boolean(init.event); - // Load base event for recurring instances + /* + * Which event this form is even about has to be settled before it opens. + * + * A form populated from the master shows the series' start date, so editing + * Wednesday's standup would offer to move Monday's — right for the series and + * wrong for one date. So the scope is asked first, and the occurrence itself + * is what the form loads when the answer is "this occurrence". + */ + /* + * Asked once per event, and deliberately not tied to the effect's lifetime. + * + * Two things make the obvious version wrong. A dialog is queued in a store + * the moment it is requested, so it outlives the effect that asked for it: a + * re-run queues a second prompt the first answer cannot retract, and the + * reader is asked the same question twice. And gating the *answer* on a + * cleanup flag is worse — React's StrictMode runs mount, cleanup, mount, so + * the flag is already set by the time anyone clicks and the editor never + * opens at all. The ref is what makes this once; the answer is applied + * whenever it arrives. + */ + const asked = useRef(null); useEffect(() => { - if (init.event?.baseEventId) void cal.getEvent(init.event.baseEventId).then((e) => setBase(e)); - else if (!init.event) setBase(null); + const ev = init.event; + if (!ev) { setBase(null); setScope("series"); return; } + if (!ev.baseEventId) { setBase(ev); setScope("series"); return; } + if (asked.current === ev.id) return; + asked.current = ev.id; + void (async () => { + const chosen = isRecurring(ev) && isOccurrence(ev) ? await askEditScope(ev) : "series"; + if (!chosen) { onClose(); return; } + setScope(chosen); + if (chosen === "occurrence") setBase(ev); + else void cal.getEvent(ev.baseEventId!).then(setBase); + })(); // eslint-disable-next-line react-hooks/exhaustive-deps }, [init.event?.id]); - if (base === undefined) return null; - return ; + if (base === undefined || scope === undefined) return null; + return ; } -function EventForm({ init, base, editing, onClose, settingsTz, defaultAlert, myEmail }: { init: EditorInit; base: CalendarEvent | null; editing: boolean; onClose: () => void; settingsTz: string; defaultAlert: number; myEmail: string }) { +function EventForm({ init, base, scope, editing, onClose, settingsTz, defaultAlert, myEmail }: { init: EditorInit; base: CalendarEvent | null; scope: EventScope; editing: boolean; onClose: () => void; settingsTz: string; defaultAlert: number; myEmail: string }) { + /** This form is editing one date rather than the series behind it. */ + const oneDate = scope === "occurrence"; const cal = useCalendar(); const contacts = useContacts(); const ev = base; @@ -175,13 +219,23 @@ function EventForm({ init, base, editing, onClose, settingsTz, defaultAlert, myE }; const invites = sendInvites && attendees.length > 0; if (ev) { + /* + * A single occurrence takes less than the series does. Four of the + * fields this form always sends are among them — `useDefaultAlerts` + * and `calendarIds` are refused outright, `recurrenceRule`, + * `privacy` and `organizerCalendarAddress` are dropped in silence — + * so they are left out here rather than sent and believed. The store + * still checks; this is what stops it having to complain. + */ + const source = oneDate + ? Object.fromEntries(Object.entries(obj).filter(([k]) => !OCCURRENCE_OMIT.has(k))) + : obj; const patch: Record = {}; - for (const [k, v] of Object.entries(obj)) patch[k] = v === undefined ? null : v; - if (Object.keys(ev.calendarIds)[0] !== calendarId) patch.calendarIds = { [calendarId]: true }; - // `ev` is the master: EventEditor resolves `baseEventId` when it opens - // on an occurrence, so the whole series is what this form edits. - await cal.updateEvent(ev, patch, invites, "series"); - toast.success("Event updated"); + for (const [k, v] of Object.entries(source)) patch[k] = v === undefined ? null : v; + if (!oneDate && Object.keys(ev.calendarIds)[0] !== calendarId) patch.calendarIds = { [calendarId]: true }; + const dropped = await runScoped(scope, (s) => cal.updateEvent(ev, patch, invites, s)); + if (!dropped) { setBusy(false); return; } + toast.success(droppedMessage(dropped) ?? (oneDate ? "This occurrence updated" : "Event updated")); } else { const clean: Record = {}; for (const [k, v] of Object.entries(obj)) if (v !== undefined) clean[k] = v; @@ -206,7 +260,13 @@ function EventForm({ init, base, editing, onClose, settingsTz, defaultAlert, myE return ( }>
- {ev && isRecurring(ev) &&
This is a recurring event — changes apply to the whole series.
} + {ev && isRecurring(ev) && ( +
+ {oneDate + ? `Editing ${formatNumericDate(start)} only — the rest of the series is unchanged. Repeat, calendar and privacy belong to the series and are not shown.` + : "This is a recurring event — changes apply to the whole series."} +
+ )}
setTitle(e.target.value)} />
{allDay ? ( @@ -231,6 +291,7 @@ function EventForm({ init, base, editing, onClose, settingsTz, defaultAlert, myE {listTimeZones().map((t) => )} )} + {!oneDate && ( + )}
- {preset === "custom" && ( + {!oneDate && preset === "custom" && (
Repeat every @@ -271,7 +333,7 @@ function EventForm({ init, base, editing, onClose, settingsTz, defaultAlert, myE )}
- setCalendarId(e.target.value)}> {calendars.map((c) => )}
@@ -329,7 +391,7 @@ function EventForm({ init, base, editing, onClose, settingsTz, defaultAlert, myE
-
+ {!oneDate &&
}