coffey-labs/ihasmail
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases 2 Wiki Activity

Calendar: never mutate an occurrence by an id we are holding

Browse Source 
Verified against the live 0.16.20 instance, which found two things the
mock had guessed wrong about.

A synthetic id encodes a position in the expanded series, and writing a
`recurrenceOverrides` entry renumbers it. A five-week series came back as
`e i m q u` over 03-01..03-29; after one override was written to 03-08
the same five ids addressed 03-01, 03-15, 03-29, 03-08 and 03-22. Nothing
was rejected. A stale id is not invalid, it is wrong - a confident answer
about the wrong day - so a delete meant for one occurrence removes
another.

`recurrenceId` is the stable name for a slot in a series, because it is
the date. `updateEvent` and `destroyEvent` now look the current id up by
it immediately before acting, and refuse outright when the date has left
the series rather than falling back to the id in hand.

The mock had this exactly backwards: it kept ids stable on purpose, which
agreed with the belief that is wrong. It now renumbers too - a different
permutation to Stalwart's, with the property that matters - and a test
holds an id across a write and watches it change meaning.

Second finding: the inherited properties are dropped *after* the server
has decided to write an override, so a patch made only of them still
writes one, carrying the server-filled start and duration and nothing
else. `{"privacy":"private"}` on one occurrence answered "updated", left
privacy untouched, and left that date with no title at all. Sending
nothing when narrowing empties a patch was written as a principle - a
request whose response could only be a meaningless "updated" is worse
than no request - and it turns out to prevent real data loss.

Both recorded in KNOWN-ISSUES with the dates they were confirmed on.
This commit is contained in:
jcoffey-dev
2026-08-30 21:39:30 -07:00
parent 5ced44ec13
commit 91481965bc
6 changed files with 166 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files
web/src/store/__tests__/event-scope.test.ts
+32 -7
View File
@@ -38,11 +38,25 @@ const MASTER: CalendarEvent = { ...OCCURRENCE, id: "i", baseEventId: undefined,
interface SetCall { update?: Record<string, unknown>; 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, unknown>, 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<string, unknown>, 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"]);
});
});
web/src/store/calendar.ts
+44 -2
View File
@@ -137,6 +137,48 @@ 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<Id> {
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<CalendarEvent> | 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.`);
@@ -488,7 +530,7 @@ export const useCalendar = create<CalendarState>((set, get) => ({
async updateEvent(event, patch, sendInvites, scope) {
const accountId = get().accountId!;
const id = eventIdForScope(event, scope);
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[] };
@@ -502,7 +544,7 @@ export const useCalendar = create<CalendarState>((set, get) => ({
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<SetResponse>("CalendarEvent/set", { accountId, destroy: [id], sendSchedulingMessages: sendInvites });
const err = res.notDestroyed?.[id];
if (err) throw new CalendarSetError(err);