fix(caldav-plugin): make recurring-occurrence edits/deletes safe and quiet #7492 (#8149)

* build: drop unused elevate.exe from Windows build #8135

elevate.exe is bundled by electron-builder's NSIS target solely so
electron-updater can apply privileged per-machine updates. We ship no
in-app auto-updater (electron-updater is not a dependency; the autoUpdater
block in electron/start-app.ts is commented out), so the binary is unused
dead weight and a frequent AV false-positive. Set packElevateHelper: false
to stop bundling it; safe because perMachine is not true.

* fix(caldav-plugin): make recurring-occurrence edits/deletes safe and quiet #7492

Expanded RRULE occurrences all share one master .ics, so operating on a
single occurrence hit the whole series: updateIssue rewrote the master
(and the next poll pulled it onto every sibling), deleteIssue deleted the
master (the entire series), and getById returned the master's DTSTART,
collapsing every instance onto the first one's time.

Plugin:
- parseCompoundId surfaces occurrenceMs instead of discarding it
- getById re-anchors start/end onto the requested occurrence (timed via
  toIcalUtcDateTime, all-day via toIcalDate matching ical.js local-midnight)
- updateIssue/deleteIssue refuse occurrence writes with a marked error
  (isExpectedSyncSkip), so the series is never mutated

Host two-way sync:
- editing or deleting a task linked to one occurrence now stays silent (the
  user changed their task, not the calendar) and the local change still
  applies. Explicit agenda reschedule/delete keep surfacing the honest
  "can't change a single occurrence" message (no false success).

Single non-recurring events are unaffected. Full per-occurrence editing
(RECURRENCE-ID overrides + EXDATE) remains a follow-up; it needs an If-Match
primitive and recurrence-value preservation.
This commit is contained in:
Johannes Millan 2026-06-08 16:05:52 +02:00 committed by GitHub
parent eff41c041d
commit aa2ad88ff1
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 389 additions and 15 deletions

View file

@ -61,6 +61,13 @@ nsis:
# allow to install to custom location
oneClick: false
allowToChangeInstallationDirectory: true
# We ship no in-app auto-updater: electron-updater is not a dependency and the
# autoUpdater block in electron/start-app.ts is commented out. elevate.exe is
# only ever invoked by electron-updater to apply privileged (per-machine)
# updates, so electron-builder bundles it here as dead weight that AV engines
# frequently false-positive on. Drop it. Safe because perMachine is not true
# (with perMachine: true, packElevateHelper: false is ignored). See #8135.
packElevateHelper: false
portable:
artifactName: ${name}-${arch}.${ext}
appx:

View file

@ -812,6 +812,201 @@ END:VCALENDAR</cal:calendar-data>
});
});
// Issue #7492 follow-up: an expanded RRULE occurrence (`#occ=<ms>` id) maps
// back to the shared master `.ics`. Write paths must NOT silently mutate the
// whole series, and getById must report the occurrence's own time.
describe('per-occurrence write safety (issue #7492)', () => {
const cfg = {
serverUrl: 'https://cloud.example.com',
username: 'user',
password: 'pass',
writeCalendarId: '/dav/calendars/user/default/',
};
const calHref = '/dav/calendars/user/default/';
const eventHref = '/dav/calendars/user/default/weekly.ics';
const occId = (ms: number): string => `${calHref}::${eventHref}#occ=${ms}`;
const masterId = `${calHref}::${eventHref}`;
const timedMaster = [
'BEGIN:VCALENDAR',
'VERSION:2.0',
'BEGIN:VEVENT',
'UID:weekly-uid',
'DTSTART:20260105T100000Z',
'DTEND:20260105T110000Z',
'RRULE:FREQ=WEEKLY;COUNT=4',
'SUMMARY:Weekly Standup',
'LAST-MODIFIED:20260101T090000Z',
'SEQUENCE:0',
'END:VEVENT',
'END:VCALENDAR',
].join('\r\n');
const allDayMaster = [
'BEGIN:VCALENDAR',
'VERSION:2.0',
'BEGIN:VEVENT',
'UID:weekly-allday-uid',
'DTSTART;VALUE=DATE:20260105',
'DTEND;VALUE=DATE:20260106',
'RRULE:FREQ=WEEKLY;COUNT=4',
'SUMMARY:Weekly Holiday',
'END:VEVENT',
'END:VCALENDAR',
].join('\r\n');
let mockHttp: any;
beforeEach(() => {
mockHttp = {
get: vi.fn().mockResolvedValue(timedMaster),
post: vi.fn(),
put: vi.fn(),
patch: vi.fn(),
delete: vi.fn(),
request: vi.fn(),
};
});
it('updateIssue refuses a single occurrence and writes nothing', async () => {
const err: any = await definition.updateIssue!(
occId(Date.parse('2026-01-12T10:00:00Z')),
{ summary: 'Changed' },
cfg as any,
mockHttp as any,
)
.then(() => null)
.catch((e) => e);
expect(err).toBeInstanceOf(Error);
expect(String(err.message)).toMatch(/single occurrence/i);
// Marker tells the host's two-way sync this is an expected limitation, not
// a failure, so editing the task stays silent (issue #7492).
expect(err.isExpectedSyncSkip).toBe(true);
// Guard runs before any network I/O — the master is never touched.
expect(mockHttp.get).not.toHaveBeenCalled();
expect(mockHttp.put).not.toHaveBeenCalled();
});
it('deleteIssue refuses a single occurrence and deletes nothing', async () => {
const err: any = await definition.deleteIssue!(
occId(Date.parse('2026-01-12T10:00:00Z')),
cfg as any,
mockHttp as any,
)
.then(() => null)
.catch((e) => e);
expect(err).toBeInstanceOf(Error);
expect(String(err.message)).toMatch(/single occurrence/i);
expect(err.isExpectedSyncSkip).toBe(true);
expect(mockHttp.delete).not.toHaveBeenCalled();
});
it('updateIssue still writes a non-occurrence (no #occ=) event', async () => {
await definition.updateIssue!(
masterId,
{ summary: 'Changed' },
cfg as any,
mockHttp as any,
);
expect(mockHttp.put).toHaveBeenCalledTimes(1);
});
it('getById re-anchors a timed occurrence to its own start/end', async () => {
const occMs = Date.parse('2026-01-19T10:00:00Z');
const issue: any = await definition.getById!(
occId(occMs),
cfg as any,
mockHttp as any,
);
expect(issue.start).toBe('20260119T100000Z');
expect(issue.end).toBe('20260119T110000Z');
// Must NOT collapse onto the master's first (Jan 5) instance.
expect(issue.start).not.toBe('20260105T100000Z');
// And it round-trips through the two-way-sync extractor to the right instant.
const vals = definition.extractSyncValues!(issue);
expect(vals.start_dateTime).toBe('2026-01-19T10:00:00.000Z');
expect(vals.duration_ms).toBe(60 * 60 * 1000);
});
it('getById re-anchors an all-day occurrence to its own date', async () => {
mockHttp.get = vi.fn().mockResolvedValue(allDayMaster);
// ical.js represents an all-day occurrence as local midnight, matching
// the value the expander stamps into the id.
const occMs = new Date(2026, 0, 19).getTime();
const issue: any = await definition.getById!(
occId(occMs),
cfg as any,
mockHttp as any,
);
expect(issue.start).toBe('20260119');
expect(issue.startParams).toBe('VALUE=DATE');
expect(issue.end).toBe('20260120');
const vals = definition.extractSyncValues!(issue);
expect(vals.start_date).toBe('2026-01-19');
});
it('getById returns the master start for a non-occurrence id', async () => {
const issue: any = await definition.getById!(masterId, cfg as any, mockHttp as any);
expect(issue.start).toBe('20260105T100000Z');
});
it('getById re-anchors a TZID-master occurrence to the right instant', async () => {
const tzidMaster = [
'BEGIN:VCALENDAR',
'VERSION:2.0',
'BEGIN:VEVENT',
'UID:weekly-tzid-uid',
'DTSTART;TZID=America/New_York:20260105T090000',
'DTEND;TZID=America/New_York:20260105T100000',
'RRULE:FREQ=WEEKLY;COUNT=4',
'SUMMARY:Weekly NY Standup',
'END:VEVENT',
'END:VCALENDAR',
].join('\r\n');
mockHttp.get = vi.fn().mockResolvedValue(tzidMaster);
// Jan 19 2026 09:00 in America/New_York (EST, UTC-5) = 14:00Z.
const occMs = Date.parse('2026-01-19T14:00:00Z');
const issue: any = await definition.getById!(
occId(occMs),
cfg as any,
mockHttp as any,
);
// The occurrence instant is preserved as UTC, regardless of the runner's TZ.
expect(issue.start).toBe('20260119T140000Z');
expect(issue.end).toBe('20260119T150000Z');
const vals = definition.extractSyncValues!(issue);
expect(vals.start_dateTime).toBe('2026-01-19T14:00:00.000Z');
// Duration comes from the TZID-resolved master (09:00->10:00 = 1h), not a
// timezone-skewed value.
expect(vals.duration_ms).toBe(60 * 60 * 1000);
});
it('getById re-anchors a floating-time master occurrence and preserves duration', async () => {
const floatingMaster = [
'BEGIN:VCALENDAR',
'VERSION:2.0',
'BEGIN:VEVENT',
'UID:weekly-floating-uid',
'DTSTART:20260105T090000',
'DTEND:20260105T103000',
'RRULE:FREQ=WEEKLY;COUNT=4',
'SUMMARY:Weekly Floating Standup',
'END:VEVENT',
'END:VCALENDAR',
].join('\r\n');
mockHttp.get = vi.fn().mockResolvedValue(floatingMaster);
const occMs = Date.parse('2026-01-19T09:00:00Z');
const issue: any = await definition.getById!(
occId(occMs),
cfg as any,
mockHttp as any,
);
expect(issue.start).toBe('20260119T090000Z');
expect(issue.end).toBe('20260119T103000Z');
const vals = definition.extractSyncValues!(issue);
expect(vals.duration_ms).toBe(90 * 60 * 1000);
});
});
describe('timeBlock.upsertEvent', () => {
const cfg = {
serverUrl: 'https://dav.example.com/',

View file

@ -62,21 +62,49 @@ const toCompoundId = (
const parseCompoundId = (
id: string,
fallbackCalendarHref: string,
): { calendarHref: string; eventHref: string } => {
): { calendarHref: string; eventHref: string; occurrenceMs?: number } => {
// Strip the occurrence suffix first so the remaining base id is unambiguous.
// A present `#occ=<digits>` marks an expanded RRULE instance whose eventHref
// still resolves to the shared master resource — surface it so write/read
// paths can stay occurrence-aware instead of silently hitting the master.
const occIdx = id.lastIndexOf(OCCURRENCE_SEP);
const baseId =
occIdx !== -1 && /^\d+$/.test(id.slice(occIdx + OCCURRENCE_SEP.length))
? id.slice(0, occIdx)
: id;
const hasOccurrence =
occIdx !== -1 && /^\d+$/.test(id.slice(occIdx + OCCURRENCE_SEP.length));
const occurrenceMs = hasOccurrence
? parseInt(id.slice(occIdx + OCCURRENCE_SEP.length), 10)
: undefined;
const baseId = hasOccurrence ? id.slice(0, occIdx) : id;
const sep = baseId.indexOf(COMPOUND_SEP);
if (sep === -1) return { calendarHref: fallbackCalendarHref, eventHref: baseId };
if (sep === -1) {
return { calendarHref: fallbackCalendarHref, eventHref: baseId, occurrenceMs };
}
return {
calendarHref: baseId.slice(0, sep),
eventHref: baseId.slice(sep + COMPOUND_SEP.length),
occurrenceMs,
};
};
/**
* A write that targets a single expanded RRULE occurrence (an `#occ=` id) can't
* be applied: the occurrence maps back to the shared master resource, so writing
* it would mutate the whole series. We refuse with this marked error. The
* `isExpectedSyncSkip` flag tells the host's two-way sync that this is an
* expected limitation the user edited/deleted their *task*, not the calendar
* so it stays silent instead of showing a sync-failure snack. Explicit calendar
* actions (agenda reschedule/delete) don't check the flag and still surface the
* message, which is the honest feedback there. See issue #7492.
*/
const unsupportedOccurrenceWriteError = (action: 'edit' | 'delete'): Error =>
Object.assign(
new Error(
`${action === 'edit' ? 'Editing' : 'Deleting'} a single occurrence of a ` +
`recurring event is not supported yet. ` +
`Please ${action} the event in your calendar app instead.`,
),
{ isExpectedSyncSkip: true },
);
// --- iCal Helpers ---
/** Unfold iCal line continuations (RFC 5545 Section 3.1) */
@ -1107,7 +1135,7 @@ PluginAPI.registerIssueProvider({
http: PluginHttp,
): Promise<PluginIssue> {
const cfg = config as unknown as CaldavCalendarConfig;
const { eventHref } = parseCompoundId(issueId, getWriteCalendarId(cfg));
const { eventHref, occurrenceMs } = parseCompoundId(issueId, getWriteCalendarId(cfg));
const eventUrl = resolveHref(cfg, eventHref);
const icalData = await http.get<string>(eventUrl, { responseType: 'text' });
const events = parseVEvents(icalData);
@ -1116,8 +1144,48 @@ PluginAPI.registerIssueProvider({
throw new Error('Event not found: ' + eventHref);
}
const startDate = parseIcalDateTime(event.dtstart, event.dtstartParams);
const endDate = parseIcalDateTime(event.dtend, event.dtendParams);
// The master VEVENT only carries the series' first DTSTART. For an expanded
// occurrence, re-anchor start/end onto THIS instance so the detail panel and
// the two-way-sync pull use the occurrence's time instead of collapsing every
// instance onto the master's. occurrenceMs is the same `toJSDate().getTime()`
// value the expander stamped into the id, so it round-trips exactly. See #7492.
let { dtstart: start, dtend: end } = event;
let startParams = event.dtstartParams;
let endParams = event.dtendParams;
if (occurrenceMs !== undefined) {
const allDay = isDateOnly(event.dtstart, event.dtstartParams);
const masterStart = parseIcalDateTime(event.dtstart, event.dtstartParams);
let durationMs = parseDuration(event.duration);
if (!durationMs && masterStart && event.dtend) {
const masterEnd = parseIcalDateTime(event.dtend, event.dtendParams);
if (masterEnd) durationMs = masterEnd.getTime() - masterStart.getTime();
}
const occStart = new Date(occurrenceMs);
if (allDay) {
// ical.js represents all-day occurrences as local midnight, so the
// local-getter formatter (toIcalDate) yields the correct calendar date.
start = toIcalDate(occStart);
startParams = 'VALUE=DATE';
if (durationMs > 0) {
const occEnd = new Date(occStart);
occEnd.setDate(occEnd.getDate() + Math.round(durationMs / 86400000));
end = toIcalDate(occEnd);
endParams = 'VALUE=DATE';
} else {
end = '';
endParams = '';
}
} else {
start = toIcalUtcDateTime(occStart);
startParams = '';
end =
durationMs > 0 ? toIcalUtcDateTime(new Date(occurrenceMs + durationMs)) : '';
endParams = '';
}
}
const startDate = parseIcalDateTime(start, startParams);
const endDate = parseIcalDateTime(end, endParams);
return {
id: issueId,
@ -1128,10 +1196,10 @@ PluginAPI.registerIssueProvider({
? parseIcalDateTime(event.lastModified, '')?.getTime()
: undefined,
summary: event.summary || '(No title)',
start: event.dtstart,
end: event.dtend,
startParams: event.dtstartParams,
endParams: event.dtendParams,
start,
end,
startParams,
endParams,
startFormatted: startDate?.toLocaleString() || '',
endFormatted: endDate?.toLocaleString() || '',
status: event.status,
@ -1237,7 +1305,9 @@ PluginAPI.registerIssueProvider({
http: PluginHttp,
): Promise<void> {
const cfg = config as unknown as CaldavCalendarConfig;
const { eventHref } = parseCompoundId(id, getWriteCalendarId(cfg));
const { eventHref, occurrenceMs } = parseCompoundId(id, getWriteCalendarId(cfg));
// Editing one occurrence would rewrite the shared master (whole series).
if (occurrenceMs !== undefined) throw unsupportedOccurrenceWriteError('edit');
const eventUrl = resolveHref(cfg, eventHref);
// Fetch current iCal data
@ -1478,7 +1548,9 @@ PluginAPI.registerIssueProvider({
http: PluginHttp,
): Promise<void> {
const cfg = config as unknown as CaldavCalendarConfig;
const { eventHref } = parseCompoundId(id, getWriteCalendarId(cfg));
const { eventHref, occurrenceMs } = parseCompoundId(id, getWriteCalendarId(cfg));
// Deleting one occurrence would DELETE the shared master (whole series).
if (occurrenceMs !== undefined) throw unsupportedOccurrenceWriteError('delete');
const eventUrl = resolveHref(cfg, eventHref);
await http.delete(eventUrl, { responseType: 'text' });
},

View file

@ -710,6 +710,48 @@ describe('IssueTwoWaySyncEffects', () => {
adapterRegistry.unregister('TEST_PROVIDER');
}));
it('should NOT show a snack when push rejects with an expected sync-skip marker (#7492)', fakeAsync(() => {
// The marker normally originates from a provider's updateIssue (e.g. a
// single recurring CalDAV occurrence); injected here via fetchIssue, which
// surfaces through the same push-pipe catch.
const expectedSkip = Object.assign(new Error('single occurrence'), {
isExpectedSyncSkip: true,
});
const adapter = createMockAdapter({
getFieldMappings: jasmine
.createSpy('getFieldMappings')
.and.returnValue([isDoneFieldMapping]),
getSyncConfig: jasmine.createSpy('getSyncConfig').and.returnValue({}),
fetchIssue: jasmine.createSpy('fetchIssue').and.rejectWith(expectedSkip),
});
adapterRegistry.register('TEST_PROVIDER', adapter);
const task = createMockTask({
id: 'task-1',
issueType: 'TEST_PROVIDER' as any,
issueId: 'issue-1',
issueProviderId: 'provider-1',
issueLastSyncedValues: { status: 'NEEDS-ACTION' },
});
taskServiceSpy.getByIdOnce$.and.returnValue(of(task));
issueProviderServiceSpy.getCfgOnce$.and.returnValue(of(createMockIssueProvider()));
effects.pushFieldsOnTaskUpdate$.subscribe();
actions$.next(
TaskSharedActions.updateTask({
task: { id: 'task-1', changes: { isDone: true } },
}),
);
tick();
expect(snackServiceSpy.open).not.toHaveBeenCalled();
adapterRegistry.unregister('TEST_PROVIDER');
}));
});
describe('pushTagChangesAfterTagDelete$', () => {
@ -895,6 +937,38 @@ describe('IssueTwoWaySyncEffects', () => {
adapterRegistry.unregister('TEST_PROVIDER');
}));
it('should NOT show a snack when delete rejects with an expected sync-skip marker (#7492)', fakeAsync(() => {
const expectedSkip = Object.assign(new Error('single occurrence'), {
isExpectedSyncSkip: true,
});
const deleteIssueSpy = jasmine
.createSpy('deleteIssue')
.and.rejectWith(expectedSkip);
const adapter = createMockAdapter({ deleteIssue: deleteIssueSpy });
adapterRegistry.register('TEST_PROVIDER', adapter);
const cfg = createMockIssueProvider();
issueProviderServiceSpy.getCfgOnce$.and.returnValue(of(cfg));
const task = createMockTask({
id: 'task-1',
issueType: 'TEST_PROVIDER' as any,
issueId: 'issue-1',
issueProviderId: 'provider-1',
}) as TaskWithSubTasks;
(task as any).subTasks = [];
effects.deleteIssueOnTaskDelete$.subscribe();
actions$.next(TaskSharedActions.deleteTask({ task }));
tick();
expect(snackServiceSpy.open).not.toHaveBeenCalled();
adapterRegistry.unregister('TEST_PROVIDER');
}));
});
describe('deleteIssueOnBulkTaskDelete$', () => {

View file

@ -43,6 +43,19 @@ const SYNCABLE_TASK_FIELDS: ReadonlySet<string> = new Set([
'tagIds',
]);
/**
* A provider's write may reject with this marker to signal an *expected*
* limitation rather than a real failure e.g. the CalDAV plugin can't yet edit
* or delete a single occurrence of a recurring event (#7492). When it does,
* two-way sync stays silent: the user changed/removed their task, not the
* calendar. Explicit calendar actions (agenda reschedule/delete) don't consult
* this and still surface the message, which is the honest feedback there.
*/
const isExpectedSyncSkipError = (err: unknown): boolean =>
typeof err === 'object' &&
err !== null &&
(err as { isExpectedSyncSkip?: boolean }).isExpectedSyncSkip === true;
const toSortedStringArray = (value: unknown): string[] =>
Array.isArray(value)
? value
@ -168,6 +181,11 @@ export class IssueTwoWaySyncEffects {
concatMap(({ fullTask, changes }) =>
this._pushChanges$(fullTask, changes).pipe(
catchError((err) => {
// Expected provider limitation (e.g. a single recurring occurrence,
// #7492) — the local task edit stands; don't alarm the user.
if (isExpectedSyncSkipError(err)) {
return EMPTY;
}
IssueLog.err('Two-way sync push failed', err);
this._snackService.open({
type: 'ERROR',
@ -214,6 +232,9 @@ export class IssueTwoWaySyncEffects {
concatMap((task) =>
this._pushChanges$(task, { tagIds: task.tagIds }).pipe(
catchError((err) => {
if (isExpectedSyncSkipError(err)) {
return EMPTY;
}
IssueLog.err('Two-way sync tag delete push failed', err);
this._snackService.open({
type: 'ERROR',
@ -488,6 +509,11 @@ export class IssueTwoWaySyncEffects {
concatMap((cfg) =>
from(adapter.deleteIssue!(issueId, cfg)).pipe(
catchError((err) => {
// Expected provider limitation (e.g. a single recurring occurrence,
// #7492) — the local task is already removed; stay silent.
if (isExpectedSyncSkipError(err)) {
return EMPTY;
}
// 404/410 means the remote issue is already gone — treat as success
// to avoid false "delete failed" toasts (e.g. when polling detects
// a remote deletion and then deleteIssue is called on the same issue)